kmarshall has asked for the wisdom of the Perl Monks concerning the following question:

Hello, I am trying to use the Image::Size module to read images in a directory. If I use this method, it will read the files in the directory of the script:
use Image::Size; for (glob '*.jpg') { my ($x, $y) = imgsize($_); print "$_: xsize = $x, ysize = $y<BR>\n"; }
However, my images are in multiple directories and it is not working when I try to use the script like this:
opendir(DIR,"../images/dir/name"); @files = readdir(DIR); close(DIR); use Image::Size; for(@files) { if ($_ =~ /.*.*\..*/) { if ($_ =~ /.jpg/ || $_ =~ /.JPG/) { my ($x, $y) = imgsize($_); print "$_: x=$x, y=$y<BR>\n"; } } }
I am not sure what I am doing wrong, but if anyone could help me, I would much appreciate it. Thanks in advance, Kyle

Replies are listed 'Best First'.
Re: Using Image::Size
by davidrw (Prior) on Sep 23, 2005 at 21:31 UTC
    You might consider using File::Find or File::Find::Rule instead (or maybe even glob).

    Just a general code comment (goes in the personal perference bucket, but i think this is "better") -- if an "if" doesn't have an "else" and takes up the entire block, it should just be a "next" (or last) call.
    use Image::Size; for(@files) { next unless $_ =~ /.*.*\..*/; next unless $_ =~ /.jpg/ || $_ =~ /.JPG/; my ($x, $y) = imgsize($_); print "$_: x=$x, y=$y<BR>\n"; }
    Also note that $_ =~ /foo/ is the same as saying /foo/. And also note the /i modifier for regexes (see perlre). Also note that the dot in the "jpg" re should be (presumably) escaped, and also presuming you want ".jpg" at the end of the string. Those, and using an "and", you can rewrite again as:
    use Image::Size; for(@files) { next unless /.*.*\..*/ && /\.jpg$/i; my ($x, $y) = imgsize($_); print "$_: x=$x, y=$y<BR>\n"; }
    Now that things are a little simplified we can go after the root problem. I'm not sure that /.*.*\..*/ does what you want.. what do you intend for it to do? As you have it, it's simply equivalent to /\./ ... My guess at what you meant would be this:
    use Image::Size; for(@files) { next unless /.{2,}\.jpg$/; # could also be just /..\.jp +g$/ my ($x, $y) = imgsize($_); print "$_: x=$x, y=$y<BR>\n"; }
    Although if you were simply trying to exclude the directories "." and "..", then /\.jpg$/i is just fine. (or even /\.jpe?g$/i). Also note that a -f test (see perlfunc) could have excluded the "." and ".." directories.
    So yet one more solution is (note use of grep instead of next):
    opendir(DIR,"../images/dir/name"); @files = grep -f && /\.jpe?g$/i, readdir(DIR); close(DIR); for(@files) { my ($x, $y) = imgsize($_); print "$_: x=$x, y=$y<BR>\n"; }

    Also, what was the behavior that you saw? Adding debugging print statements before and after the "next" statements will probably help you see what it is doing ..
      After trying all of these solutions, none of them seemed to work. It gave me the same results, which is a list of the files in the directory, but with nothing for the sizes. The variables are empty I guess.

      Thanks for helping, I appreciate it.

      Kyle
Re: Using Image::Size
by Tanktalus (Canon) on Sep 23, 2005 at 23:16 UTC

    Rule #2 of coding is "Are you testing what you think you're testing?" Quick check would be to insert the following two lines right after your close(DIR):

    use Data::Dumper; print Dumper(\@files);
    You'd then notice that "../images/dir/name" is not part of the files elements, so when you call imgsize, it's not going to have the faintest idea what file you're referring to.

    This is actually a place where glob is great. Just convert your first three lines to:

    @files = glob '../images/dir/name/*.{jpg,JPG}';
    and you can pretty much get rid of your if's.

    Update: Rule #2a of coding is "Are you testing what you're posting?" <sigh> - added the Dumper function call as nedals points out.

      I inserted the two lines after the close(DIR) for a snippet of the following:
      opendir(DIR,"../images/dir/name"); @files = readdir(DIR); close(DIR); use Data::Dumper; print \@files;
      This gave me the following output: ARRAY(0x8057b2c)

      I am new to Perl, and I am not sure what this means. I then tried your second suggestion, for a code of the following:
      @files = glob '../images/dir/name/*.{jpg,JPG}'; use Image::Size; for(@files) { my ($x, $y) = imgsize($_); print "$_: x=$x, y=$y<BR>\n"; }
      This gave me an output of: ../images/dir/name: x=, y=

      I am still a little confused about what to try next, so if anyone has any suggestions, I would appreciate it.

      Thanks so much for the help,
      Kyle

        Ok, knowing your skill level is always a plus on how we approach helping you. So we'll take it in smaller steps here. First question: is the directory name right? If you run "dir ..\images\dir\name" if Windows or "ls ../images/dir/name" if Linux/unix, do you see the files you want to work with? If not, you need to convert to using the right directory names.

        There are ways to do this dynamically as something relative to where your script lives, but let's not cover too much ground in a single node here, so it may just be easier to put in the full path name such as "C:/images/dir/name" on Windows or "/home/kmarshall/images/dir/name" on Linux/unix. We can cover doing this in other ways at another time.

        Ok, now next step. Just put the following in your program:

        @files = glob '/home/kmarshall/images/dir/name/*'; use Data::Dumper; print Dumper(\@files);
        and tell us what it says. Of course, put in the right path name that applies to where your images are.

        Once we have gotten that far, it will be much easier to continue on from there.

        What you are printing is a reference to the array
        It should be..

        print Dumper(\@files);