Is this too cumbersome?

Not for 'how-to' coding questions but PHP theory instead, this forum is here for those of us who wish to learn about design aspects of programming with PHP.

Moderator: General Moderators

Theory?
Forum Contributor
Posts: 138
Joined: Wed Apr 11, 2007 10:43 am

Is this too cumbersome?

Post by Theory? »

~pickle | Please use [ code=html ], [ code=php ], etc tags where appropriate when posting code. Your post has been edited to reflect how we'd like it posted. Please read: :arrow: Posting Code in the Forums to learn how to do it too.


So I'm working on an image gallery and I have it set up as follows.

I have a generic Image object which knows the height, width, orientation, mime-type, etc. of a photo. The object is given a directory linking to the photo as well as things like the title, author, and an array of EXIF info.

I thought about how I wanted to handle thumbnails and it seemed like creating two strategy objects that implement the same ImageStyle interface would be a good way. Each Image object has two methods, makeThumb() and makeFull() which are each passed an instance of any ImageStyle object which are in turn given the instance of the Image object to which it was passed. This way, should the actual means of creating a thumbnail or full view of the photos in the gallery ever change, all I need to do is swap out those strategy objects and the rest of the module will remain in tact and functional.

However, I've stumbled upon a strange conundrum. In order to make the thumbs appear I did this:

Code: Select all

<?php
    while($row = mysqli_fetch_assoc($rs)) {
        
        $file = $row['fileName'];
        $dir = './galleries/' . strtolower($row['category']) . '/';
        $dir .= $file;
        $title = $row['title'];
        $author = $row['author'];
        
        echo "<img src=\"getThumb.php?file=./$dir&title=$title
            &author=$author\" id=\"thumbnail\" />";
    }
?>
The getThumb.php script is really simple, it makes an Image object and calls the makeThumb method, passing it the ImageStyle object:

Code: Select all

<?php
require_once 'Image.php';
require_once 'SquareCropThumb.php';
 
$file = $_GET['file'];
$title = $_GET['title'];
$author = $_GET['author'];
 
$img = new Image($file, $title, $author);
$mime = $img->getMime();
$img->makeThumb(new SquareCropThumb());
 
$thumb = $img->getThumb();
 
switch($mime) {
    
    case 'image/jpeg':
        header('Content-Type: image/jpeg');
        imagejpeg($thumb, NULL, 80);
        break;
    
    case 'image/gif':
        header('Content-Type: image/gif');
        imagegif($thumb);
        break;
        
    case 'image/png':
        header('Content-Type: image/png');
        imagepng($thumb);
        break;
        
    default:
        echo 'File is not an image';
        break;
    
}
 
?>
But it just dawned upon me that in order to build the full-size images, I would essentially have to create new instances of each Image, and I fear that may get unwieldy as time goes on.

Am I going about this all wrong, or is my solution not as bad as I think it might be?


~pickle | Please use [ code=html ], [ code=php ], etc tags where appropriate when posting code. Your post has been edited to reflect how we'd like it posted. Please read: :arrow: Posting Code in the Forums to learn how to do it too.
User avatar
pickle
Briney Mod
Posts: 6445
Joined: Mon Jan 19, 2004 6:11 pm
Location: 53.01N x 112.48W
Contact:

Re: Is this too cumbersome?

Post by pickle »

I'm not sure in what context either of those files are. However, your approach doesn't seem all that cumbersome (though maybe I'm not understanding fully).

What user action is triggering the generation of the thumbnails? Is the user requesting a gallery page, which then generates all the thumbnails? Couldn't you generate the full image at the same time? Can't you use the same Image object to generate both the thumbnail and the full image?
Real programmers don't comment their code. If it was hard to write, it should be hard to understand.
Theory?
Forum Contributor
Posts: 138
Joined: Wed Apr 11, 2007 10:43 am

Re: Is this too cumbersome?

Post by Theory? »

Here is the entire gallery.php file:

Code: Select all

<?php
 
require 'Config.php';
 
function __autoload($class) {
    
    $file = $class . '.php';
    
    if(file_exists($file)) {
        
        require $file;  
    }
 
}
    
    if(!isset($_GET['catID'])) {
        
        /**
            Send user to gallery select page
        */
    } else {
        
        $catID= $_GET['catID'];
    }
    
    try {
        
        $mySQL = MySQLConnect::getInstance(HOST, USERNAME, PASSWORD);
    } catch(MySQLException $e) {
        
        echo $e;
        exit();
    }
    
    /**
        Get the name of the category and the image filenames, titles, and author sorted by their ID's
    */
    
    $strSQL = "SELECT fileName, title, author, category FROM photos AS p, authors AS auth, categories AS cat, photo_associations AS pa WHERE cat.cat_id = pa.cat_id AND auth.author_id = pa.author_id AND p.photo_id = pa.photo_id ORDER BY p.photo_id";
    
    $fetch = $mySQL->fetchResultSet($strSQL, DB);
    
    try{
    
        $rs = $fetch->getResultSet();
    } catch(MySQLException $e) {
        
        echo $e;
        exit();
    }
    
    echo '<div id="galleryContainer">';
    
    echo '<div id="thumbSpace">';
    
    while($row = mysqli_fetch_assoc($rs)) {
        
        $file = $row['fileName'];
        $dir = './galleries/' . strtolower($row['category']) . '/';
        $dir .= $file;
        $title = $row['title'];
        $author = $row['author'];
        
        echo "<img src=\"getThumb.php?file=./$dir&title=$title
            &author=$author\" id=\"thumbnail\" />";
    }
    
    echo '</div>';
    
    echo '</div>';
 
?>
This is a gallery module in the site that will display thumbnails that link to full size images which will show up in some other place. This particular site will have them appear in a neighboring <DIV> using some sort of ajax thing, I haven't decided exactly how I want to do it yet. Either way, the Images class has two separate methods:

Code: Select all

<?php
    public function makeThumb(ImageStyle $thumbstyle) {
        
        $this->thumb = $thumbstyle->fetchImage($this);
    }
    
    public function makeFull(ImageStyle $imgstyle) {
        
        $this->thumb = $thumbstyle->fetchImage($this);
    }
?>
I posted how the Image classes are made, it's not exactly a factory, it's just a page that get's implemented over and over again, I may try to refine it later, but I wanted to have something that worked for the time being.

The SquareCropThumb class implements the ImageStyle interface.

What isn't in the code is how I want to make the full-size images. I want the thumbnails to be links that will make the full-size images appear in a neighboring <div> and it seems to me like the only way to do that would be to make a getFull.php file that builds all new Image objects and calls the makeFull() methods at a separate time. I'm just wondering if this would become too weighty if the gallery has a lot of images to display.
User avatar
pickle
Briney Mod
Posts: 6445
Joined: Mon Jan 19, 2004 6:11 pm
Location: 53.01N x 112.48W
Contact:

Re: Is this too cumbersome?

Post by pickle »

To answer your original question now that I know how you're doing things: yes, it is too cumbersome. The thumbnails and full images should only ever be made once, then stored on the filesystem. Subsequent views of the gallery shouldn't require regeneration.

Rather than calling makeThumb() every time, I'd just call getThumb(). In getThumb() then, it checks if the thumbnail file already exists. If the image file does exist, it simply opens the image & passes the data. If not, getThumb() calls makeThumb() first, then passes the data after it's made.

I'd do the same thing for when you want to view the full image.
Real programmers don't comment their code. If it was hard to write, it should be hard to understand.
Theory?
Forum Contributor
Posts: 138
Joined: Wed Apr 11, 2007 10:43 am

Re: Is this too cumbersome?

Post by Theory? »

Nice, well that was sort of what I was looking for. In respect to what you said though, what do I do when the requirements for the thumbnails and full-size images changes? For instance, what do I do once I want the script to make full-scale thumbnails or add a watermark to or change the size of the full-size view?
User avatar
pickle
Briney Mod
Posts: 6445
Joined: Mon Jan 19, 2004 6:11 pm
Location: 53.01N x 112.48W
Contact:

Re: Is this too cumbersome?

Post by pickle »

2 options:
  1. Delete all the thumbnails & full size images (but not the originals - it sounds like those are untouched), modify your functions & re-view the gallery. That won't be any more overhead that what you're doing now.
  2. Better option: Have a 'force' option that you can toggle on & off in your class. Set it to TRUE and make your getThumb() (and supposed getFullSize()) functions call makeThumb() and makeFullSize() if 'force' is set to TRUE, regardless of whether or not the image exists or not. Make your changes to the functions then re-view the gallery. This also will have no more overhead than what you're currently doing, and also means you don't have to manually delete the files (and there's no chance a user will get a broken image on their page).
Real programmers don't comment their code. If it was hard to write, it should be hard to understand.
Theory?
Forum Contributor
Posts: 138
Joined: Wed Apr 11, 2007 10:43 am

Re: Is this too cumbersome?

Post by Theory? »

So now I have this:
while($row = mysqli_fetch_assoc($rs)) {

$file = $row['fileName'];
$dir = './galleries/' . strtolower($row['category']);
$title = $row['title'];
$author = $row['author'];

if(!file_exists($dir . '/thumbs/' . $file) || FORCE == TRUE) {

echo "<img src=\"getThumb.php?dir=$dir&file=$file&title=$title
&author=$author\" id=\"thumbnail\" />";
} else {

echo "<img src=\"$dir/thumbs/$file\" id=\"thumbnail\" />";
}
}

And that works just fine even when I set FORCE to false, which is a nice idea, so thank you for that.

I guess what this means I have to make the thumbnails and full-size views on upload, right?
Theory?
Forum Contributor
Posts: 138
Joined: Wed Apr 11, 2007 10:43 am

Re: Is this too cumbersome?

Post by Theory? »

Just in case a thumbnail isn't made yet, is there a way I can get imagejpeg() to save the file AND output it to the browser?

I tried things like:
return imagejpeg($foo, '/path/to/file', 80);

and

echo imagejpeg($foo, '/path/to/file', 80);

and those don't work. I can't make a second line either to do it again.
User avatar
pickle
Briney Mod
Posts: 6445
Joined: Mon Jan 19, 2004 6:11 pm
Location: 53.01N x 112.48W
Contact:

Re: Is this too cumbersome?

Post by pickle »

The docs are pretty clear - imagejpeg() returns a boolean.

I would put the logic of determining whether or not to generate a thumbnail, inside the getThumb() function, rather than in your gallery script. It's certainly a personal choice, but I think it'd be fine if you use getThumb.php all the time & have the getThumb() function possibly generate the thumbnail, then use readfile() to output the data.
Real programmers don't comment their code. If it was hard to write, it should be hard to understand.
josh
DevNet Master
Posts: 4872
Joined: Wed Feb 11, 2004 3:23 pm
Location: Palm beach, Florida

Re: Is this too cumbersome?

Post by josh »

imagestring()
Theory?
Forum Contributor
Posts: 138
Joined: Wed Apr 11, 2007 10:43 am

Re: Is this too cumbersome?

Post by Theory? »

I've come up with a slightly more elegant soultion based on your suggestions:

Code: Select all

<?php
require_once 'Image.php';
require_once 'SquareCropThumb.php';
 
class ThumbnailBuilder {
    
    public function buildImage($dir, $file, $title, $author) {
    
        $image = $dir;
        $image .= '/' . $file;
    
        $img = new Image($image, $title, $author);
        $mime = $img->getMime();
        $img->makeThumb(new SquareCropThumb());
    
        $thumb = $img->getThumb();
 
        switch($mime) {
    
            case 'image/jpeg':
                imagejpeg($thumb, $dir . '/thumbs/' . $file, 80);
                break;
    
            case 'image/gif':
                imagegif($thumb, $dir . '/thumbs/' . $file);
                break;
            
            case 'image/png':
                imagepng($thumb, $dir . '/thumbs/' . $file, 0);
                break;
        
            default:
                echo 'File is not an image';
                break;
        
        }
        
        imagedestroy($thumb);
    }
}
 
?>
Then in the gallery we have:

Code: Select all

<?php
    while($row = mysqli_fetch_assoc($rs)) {
        
        $dir = './galleries/' . strtolower($row['category']);
        $file = $row['fileName'];
        $title = $row['title'];
        $author = $row['author'];
        
        if(!file_exists($dir . '/thumbs/' . $file) || FORCE == TRUE) {
        
            $thumb = new ThumbnailBuilder();
            $thumb->buildImage($dir, $file, $title, $author);
        }
            
            echo "<img src=\"$dir/thumbs/$file\" id=\"thumbnail\" />";
    }
?>
User avatar
pickle
Briney Mod
Posts: 6445
Joined: Mon Jan 19, 2004 6:11 pm
Location: 53.01N x 112.48W
Contact:

Re: Is this too cumbersome?

Post by pickle »

I'd still personally put the logic of deciding whether or not to build the thumbnail, inside the thumbnail generation class. That's a personal choice - I don't think yours is necessarily bad.

The only thing I'd suggest is rather than checking against 'image/jpeg', 'image/gif', and 'image/png', you use the respective IMAGETYPE_XXX constants (here's a list of what the constants equate to). Also, to create standards compliant html, you can't have each thumbnail have the same id. Maybe you want 'class' instead?
Real programmers don't comment their code. If it was hard to write, it should be hard to understand.
User avatar
inghamn
Forum Contributor
Posts: 174
Joined: Mon Apr 16, 2007 10:33 am
Location: Bloomington, IN, USA

Re: Is this too cumbersome?

Post by inghamn »

Instead of writing PHP code whether to create and cache the thumbnail, I have my photo album link directly to the thumbnail file as if it were there. Then, I have a custom Apache 404 handler that directs image request 404s to a php script that generates a single thumbnail, then redirects back to the URL the thumbnail should be at.

http://sourceforge.net/projects/photodatabase

Here's a link to the source of the Media class that handles all the thumbnail generation
http://photodatabase.svn.sourceforge.ne ... iew=markup
Theory?
Forum Contributor
Posts: 138
Joined: Wed Apr 11, 2007 10:43 am

Re: Is this too cumbersome?

Post by Theory? »

inghamn wrote:Instead of writing PHP code whether to create and cache the thumbnail, I have my photo album link directly to the thumbnail file as if it were there. Then, I have a custom Apache 404 handler that directs image request 404s to a php script that generates a single thumbnail, then redirects back to the URL the thumbnail should be at.

http://sourceforge.net/projects/photodatabase

Here's a link to the source of the Media class that handles all the thumbnail generation
http://photodatabase.svn.sourceforge.ne ... iew=markup
I feel like problems would then arise if a 404 happened elsewhere on the site, no?
pickle wrote:I'd still personally put the logic of deciding whether or not to build the thumbnail, inside the thumbnail generation class. That's a personal choice - I don't think yours is necessarily bad.

The only thing I'd suggest is rather than checking against 'image/jpeg', 'image/gif', and 'image/png', you use the respective IMAGETYPE_XXX constants (here's a list of what the constants equate to). Also, to create standards compliant html, you can't have each thumbnail have the same id. Maybe you want 'class' instead?
I used those initially, but I realized that if I wanted to have a variable header, should one be needed, I can still rely on the method from the Image class to simply provide that instead of me having to do the translation later. Is there any substantial performance gains to using the IMAGETYPE constants?
User avatar
inghamn
Forum Contributor
Posts: 174
Joined: Mon Apr 16, 2007 10:33 am
Location: Bloomington, IN, USA

Re: Is this too cumbersome?

Post by inghamn »

Theory? wrote: I feel like problems would then arise if a 404 happened elsewhere on the site, no?
No, not at all. You would only apply the ErrorDocument command to the directory where you store all your images.
Apache's conf would have something like:

Code: Select all

 
<Directory "/path/to/application_name/html/rolls">
    ErrorDocument 404 /photobase/media/404.php
</Directory>
 
Post Reply