Well, overall, it's a pretty good script, but since you encouraged advanced critique, there are a few things you might want to change.

First, your script crashes if it encounters a non-JPEG file. I never understood why you used substr($file, -3, 3)=="php" instead of substr($file, -4)==".jpg".

Second, don't use scandir(), it's PHP 5 only. opendir, readdir, closedir works just as well.

Third, using $_GET isn't a really good idea unless the script is protected elsewhere, since someone could DoS your server by running that script repeatedly once they found it.

Fourth, you should sanitize your input; make sure size is positive, make sure color is a color, etc.

Fifth, why didn't you combine statements? if ($file == ".") continue; if ($file == "..") continue; can easily be shortened to if ($file=='.'||$file=='..') continue;, which in turn can be shortened to if (substr($file,0,1)=='.') continue, since any UNIX file that starts with . should not be touched (for instance, .HTACCESS).

Here's my rewritten version of it.

[link]

It's kind of sparse on comments, so if you don't understand something, feel free to ask.

The improvements are:

- faster, smaller
- transparent backgrounds
- function interface
- different naming scheme
- works for all images, not just JPEGs
- doesn't crash on non-PHP non-JPEG files
- doesn't crash on invalid image files

Consider creating a method which will resize any gif, jpg, or png image and utilizing that here - that will give your code more power, since it can handle multiple filetypes, and also make it more reusable.

--
Check out my Website Development Blog.

Thank you

Cheers =genstones. I concentrated on jpegs as I'm working through the tutorials in .net.

How's mkthumb()?

Looks good

--
Check out my Website Development Blog.

Nice code, but you should really be filtering those $_GET calls for malicious code.

--
_{The DataGrid control is intended for viewing data, and not as a layout tool like an HTML table - Adobe}

Very, very nice work. Good to see things like hexadecimal right shifts. My only comment is that `mkthumbdir` does not appear to return false, if `mkthumb` returns false. Unless I'm reading something wrong, `mkthumbdir` always returns true, and thus `mkthumbdir() or die('Error.';` will never error?

--
camendesign.com - code is art

Thanks muchly, though the context for which is was written (a quick and dirty thing to generate thumbnails without having to wade through GIMP for each one) meant that it was run once then deleted.

Should I use it in anger then you are of course correct.