tags:

views:

218

answers:

6

I have the php code below which help me get a photo's thumbnail image path in a script

It will take a supplied value like this from a mysql DB '2/34/12/thepicture.jpg' It will then turn it into this '2/34/12/thepicture_thumb1.jpg'

I am sure there is a better performance way of doing this and I am open to any help please

Also on a page with 50 user's this would run 50 times to get 50 different photos

// the photo has it is pulled from the DB, it has the folders and filename as 1
$photo_url = '2/34/12/thepicture_thumb1.jpg';
//build the full photo filepath
$file = $site_path. 'images/userphoto/' . $photo_url;
// make sure file name is not empty and the file exist 
if ($photo_url != '' && file_exists($file)) {
    //get file info
    $fil_ext1 = pathinfo($file);
    $fil_ext = $fil_ext1['extension'];
    $fil_explode = '.' . $fil_ext;
    $arr = explode($fil_explode, $photo_url);
    // add "_thumb" or else "_thumb1" inbetween 
    // the file name and the file extension 2/45/12/photo.jpg becomes 2/45/12/photo_thumb1.jpg
    $pic1 = $arr[0] . "_thumb" . $fil_explode;
    //make sure the thumbnail image exist
    if (file_exists("images/userphoto/" . $pic1)) {
     //retunr the thumbnail image url
     $img_name = $pic1;
    }
}

1 thing I am curious about is how it uses pathinfo() to get the files extension, since the extension will always be 3 digits, would other methods of getting this value better performance?

A: 

$img_name = preg_replace('/^(.*)(\..*?)$/', '\1_thumb\2', $file);

Edit: bbcode disappeared with \.

Havenard
although this would combine two lines of code into one, using regular expressions for something as simple as this is not really a good idea.
Josiah
...plus you're missing a dot
nickf
Now you have many problems ;) Seriously though, regex is unnecessary and too complex for this problem.
DisgruntledGoat
Oh, the bbcode formatation disappeared with my dot...
Havenard
A: 

Why are you even concerned about the performance of this function? Assuming you call it only once (say, when the "main" filename is generated) and store the result, its runtime should be essentially zero compared to DB and filesystem access. If you're calling it on every access to re-compute the thumbnail path, well, that's wasteful but it's still not going to be significantly impacting your runtime.

Now, if you want it to look nicer and be more maintainable, that's a worthwhile goal.

Andrew Medico
yes, even if this was very inefficient code, it probably would not a massive bottleneck for his system, but *there's nothing wrong with wanting to write better and more efficient code!!*
nickf
@nickf There's nothing wrong with wanting to write better and more-efficient code, but in the big picture, it's very unlikely that improving the efficiency of *this* code will make a significant difference in the overall quality and efficiency of the code in the system as a whole. To borrow a common colloquialism: there's nothing wrong with wanting your deck chairs to be arranged nicely, but rearranging deck chairs on the Titanic is not only useless, it's counterproductive.
Imagist
@nickf (continued) Given that this code is an illegible mess, its performance is a small concern in comparison (unless profiling shows otherwise).
Imagist
There certainly can be something wrong with optimizing code that doesn't need it: the opportunity cost of not working on areas that actually need attention.
Andrew Medico
+7  A: 

Is there a performance problem with this code, or are you just optimizing prematurely? Unless the performance is bad enough to be a usability issue and the profiler tells you that this code is to blame, there are much more pressing issues with this code.

To answer the question: "How can I improve this PHP code?" Add whitespace.

Imagist
+2  A: 

The best optimization for this code is to increase it's readability:

// make sure file name is not empty and the file exist 
if ( $photo_url != '' && file_exists($file) ) {

    // Get information about the file path
    $path_info = pathinfo($file);

    // determine the thumbnail name 
    // add "_thumb" or else "_thumb1" inbetween 
    // the file name and the file extension 2/45/12/photo.jpg
    // becomes 2/45/12/photo_thumb.jpg
    $pic1 = "{$path_info['dirname']}/{$path_info['basename']}_thumb.{$fil_ext}";

    // if this calculated thumbnail file exists, use it in place of
    // the image name
    if ( file_exists( "images/userphoto/" . $pic1 ) ) {
        $img_name = $pic1;
    }
}

I have broken up the components of the function using line breaks, and used the information returned from pathinfo() to simplify the process of determining the thumbnail name.

Updated to incorporate feedback from @DisgruntledGoat

Josiah
The pathinfo function returns the variables that you get from `dirname` and `basename`. Also you're not adding the _thumb1 as far as I can tell.
DisgruntledGoat
Enhanced readability is not an optimization.
slypete
Thanks to @DisgruntledGoat for the feedback, your right and I've modified the answer accordingly@slypete - I disagree, in fact as far as cost goes maintenance is one of the most important factors, and enhanced readability results in less time required for a maintainer to understand the code.
Josiah
+5  A: 

Performance-wise, if you're calling built-in PHP functions the performance is excellent because you're running compiled code behind the scenes.

Of course, calling all these functions when you don't need to isn't a good idea. In your case, the pathinfo function returns the various paths you need. You call the explode function on the original name when you can build the file name like this (note, the 'filename' is only available since PHP 5.2):

$fInfo = pathinfo($file);
$thumb_name = $fInfo['dirname'] . '/' . $fInfo['filename'] . '_thumb' . $fInfo['extension'];

If you don't have PHP 5.2, then the simplest way is to ignore that function and use strrpos and substr:

// gets the position of the last dot
$lastDot = strrpos($file, '.');
// first bit gets everything before the dot,
// second gets everything from the dot onwards
$thumbName = substr($file, 0, $lastDot) . '_thumb1' . substr($file, $lastDot);
DisgruntledGoat
thank you, eventually I might just restructure and store 2 fields in the DB, the folder path and the image name seperate, then add the thumb part to the beggining, I think that would be best perforamnce on a page with 50 photos
jasondavis
A: 

The easiest way to fix this is to thumbnail all user profile pics before hand and keep it around so you don't keep resizing.

they are resized, there are several sizes of thumbnails saved, this function gets the size that I need
jasondavis