Perl Monk, Perl Meditation | |
PerlMonks |
Re: JPEG Files ReSizeby John M. Dlugosz (Monsignor) |
on Aug 25, 2001 at 12:38 UTC ( [id://107807]=note: print w/replies, xml ) | Need Help?? |
Nice--I never realized that GD could do all that. I recall it using 8-bit paletted color only. Looks like it might be a light-duity replacement for Image::Magick now? Some comments:
Why are you interpolating that into an otherwise empty string? You are using it in an ordinary way, so I don't see any point to it. How about writing lines 59-63 as on 65-66, You leave @tmp laying around even though you only needed it for the next line. There are various ways of not doing that. But, look at more specific ways of getting the basename (see File::Basename module). But since $name is never used anyway, why bother at all? Add use warnings; along with the use strict;. Replace comment block on lines 1-11 with POD. On line 18, if ( @_ ) { wraps the entire function. So, if passed no arguments it does nothing with no error? What if there's only 1 parameter? The check doesn't seem very useful. And if you do make it, don't wrap the whole thing like that, instead do something like Also in that function, my $file = "$_[0]"; my $newWidth = $_[1]; again, what's with the quotes (sometimes)? The usual way is: Ugh: my $newFile = "new".$file; try replacing 21-22 with simply that is, interpolate $file right where you need it. And don't forget the or croak "Cannot open [new$file] for writing. OS says $!"; Always follow open with "or die..." or other check. Take a look at if ( $_[3] ) { $quality = $_[3] if ( $_[3] =~ /[1..100]/ ) ; } If you're checking the range to be between 1 and 100 number, that won't do it! [1..100] is a character class, meaning the same as [10.] or matching any character that's a 1, 0, or dot. You will pass the test if your string contains any of those characters somewhere. So 43 fails, but "hi." passes. Give it a name, not $_[3]. Then, don't just silently ignore it if it doesn't match your needs!
So, to rewrite that part, follow me: On lines 38-44, you correct the aspect ratio by changing the requested height to be consistant with the width, only if it wasn't square to begin with. You say if it was square, go ahead and allow arbitrary resizing? If that's a peculuarity of your problem, it should be noted in the comments at the top. If you remove the check, why even have the user specify a height? You always ignore it and figure it from the new width. Leave it out, or get fancier (ask me when you're ready). On line 72, don't put the & before the function name. That's really old syntax. For the pattern, japhy can tell you more about patterns. But I know that the leading .* is useless. Even without, it means the same thing: match if ".jpg" is present anywhere in the string. I think you want /\.jpg$/, to match the end. Better yet, /\.jpe?g$/i. Again, you silently do nothing but ignore the request if the file doesn't end in .jpg. It also seems to me that instead of doing tests to assure that the name's directoryness matches the -a/-d parameter, why not use the -f test to decide? That is, if the user give a file name do the file, if he gives a directory name do the directory! No need to have the first parameter in your script.
Nice trick on the grep -f to get files. Why not do the same thing for the .jpg extension, instead of using the if statement?
I hope that's helpful. I went over this in detail since you said you are learning, and learning by jumping in and doing is the way I like best! Keep at it. —John
In Section
Craft
|
|