0

I wrote a program to compare the image files of two folders(having 1000 files each) with some logic (see this SO question).

While executing it is comparing successfully until 900 images but then it gives an error like Use of uninitialized value within @tfiles2 in concatenation (.) or string at C:\dropbox\Image_Compare\image_magick.pl line 55 (#3). And then I get a popup error like Perl Command Line Interpreter has stopped working, so I close the program.

My code is as follows:

#!/usr/bin/perl
 use Image::Magick;
 no warnings 'uninitialized';
 use warnings;
 use diagnostics;
#use strict;
 use List::Util qw(first);

    my $directory1="C:/dropbox/Image_Compare/folder1";
    opendir(DIR, $directory1) or die "couldn't open $directory1: $!\n";
    my @files1 = grep { (!/^\./) && -f "$directory1/$_" } readdir(DIR);
    closedir DIR;
    print @files1;
    print 'end of files1';
    my $directory2="C:/dropbox/Image_Compare/folder2";
    opendir(DIR, $directory2) or die "couldn't open $directory2: $!\n";
    my @files2= grep { (!/^\./) && -f "$directory2/$_" } readdir(DIR);
    closedir DIR;
    print @files2;
    print 'end of files2';
    print $files1[0];
    foreach my $fils2 (@files2)
    {
        $g1 = Image::Magick->new;
        $g2 = Image::Magick->new;
        $temp1 = $g1->Read( filename=>"C:/dropbox/Image_Compare/folder1/".$files1[0].""); 
        $temp1 = $g2->Read( filename=>"C:/dropbox/Image_Compare/folder2/".$fils2."");
        $g3 = $g1->Compare( image=>$g2, metric=>'AE' ); # compare
        $error1 = $g3->Get( 'error' );
        #print $error1;
        if ($error1 == '0') 
        {
            print "Matching image is:"; 
            print $fils2 . "\n"; 
            my $tdirectory2="C:/dropbox/Image_Compare/folder2";
            opendir(DIR, $tdirectory2) or die "couldn't open $directory2: $!\n";
            my @tfiles2 = grep { (!/^\./) && -f "$tdirectory2/$_" } readdir(DIR);
            closedir DIR;
            #my $index = firstidx { $_ eq'"' .$fils2.'"' } @tfiles2;
            my $index = first { $tfiles2[$_] eq $fils2} 0..$#tfiles2;
            #print $fils2;
            print $index;
            my $i=0;
            foreach my $fils1 (@files1)
            {
                print 'ganesh';
                print $files1[$i];
                print $tfiles2[$index];
                print 'gowtham'; print "<br />";
                #print @tfiles2;
                $g4 = Image::Magick->new;
                $g5 = Image::Magick->new;
                $temp2 = $g4->Read( filename=>"C:/dropbox/Image_Compare/folder1/".$files1[$i].""); 
                $temp2 = $g5->Read( filename=>"C:/dropbox/Image_Compare/folder2/".$tfiles2[$index]."");
                $g6 = $g4->Compare( image=>$g5, metric=>'AE' ); # compare
                $error2 = $g6->Get( 'error' );
                $i++;
                $index++;
                if ($error2 == '0') {}
                else {print "Image not matching:"; print $tfiles2[$index]; last;}
                #if ($i == '800') {last;}


            }
            last
        }
    }

Can anyone please help, where i am doing a mistake.

Folder 1 file names: 0025.bmp to 1051.bmp;

Folder 2 file names: 0000.bmp to 1008.bmp;

Thanks Ganesh

Community
  • 1
  • 1
  • 1
    first impression: use Carp::Always, it may help for help:) – gaussblurinc Dec 18 '12 at 12:50
  • 1
    you might have a corrupt image file? Have you tried with a different set of images? Does it always stop at exactly the same place? – foundry Dec 18 '12 at 12:54
  • 4
    This code could use some refactoring; seperating the code into easily testable subroutines would improve readability and testability, while reducing the chance of simple typos. (Loading two images and comparing them, then getting the error is a common operation; as is loading a directory.) Also, turn `strict` back on. – amon Dec 18 '12 at 12:56
  • Why are you adding `.""` in this expression: `"C:/dropbox/Image_Compare/folder2/".$fils2.""` It makes no sense to add an empty string to the end of the string you've built. – Andy Lester Dec 18 '12 at 14:41

1 Answers1

2

I don't know which the offending line is, but one of these is likely to be the candidate:

$temp2 = $g5->Read( filename=>"C:/dropbox/Image_Compare/folder2/".$tfiles2[$index]."");

or

else {print "Image not matching:"; print $tfiles2[$index]; last;}

Do note that you increment $index whether or not it is inside the array bounds. You do not check for the condition $index > $#tfiles, which should break the loop.

You might want to assert that both input arrays contain >> 900 elements, by printing the length like print "length: ", scalar @array, "\n";.

You can test at which index the undefined error actually happens by testing for definedness of the elements in the arrays:

if (not defined $tfiles[$index] or not defined $files1[$i]) {
   die "There was an undefined element at index=$index, i=$i";
}

But then again, the offset between $i, and $index is constant (as mentioned in my answer), so you don't have to actually carry two variables.

A simple comparator subroutine could make your code more readable, thus aiding debugging (see procedural programming).

# return true if matching, false otherwise.
sub compare_images {
   my ($file1, $file2) = @_;
   my $image1 = Image::Magick->new;
   $image1->Read(filename => $file1);
   my $image2 = Image::Magick->new;
   $image2->Read(filename => $file2);
   my $result = $image1->Compare(image => $image2, metric => 'AE')->Get('error');
   # free memory
   undef $image1;
   undef $image2;
   return 0 == $result;
}

called like

my $image_root = "C:/dropbox/Image_Compare";
my ($folder1, $folder2) = qw(folder1 folder2);
unless (compare_images("$image_root/$folder1/$files1[$i]", 
                       "$image_root/$folder2/$tfiles[$index]")) {
   print "Images not matching at index=$index, i=$i\n";
   print "filename: $tfiles[$index]\n";
   last;
}

You could read your directories like

sub get_images_from_dir {
   my ($dirname) = @_;
   -d $dirname or die qq(The path "$dirname" doesn't point to a directory!);
   opendir my $dir => $dirname or die qq(Can't open "$dirname": $!);
   my @files = grep {!/^\./ and -f "$dirname/$_"} readdir $dir;
   closedir $dir;
   unless (@files) { die qq(There were no interesting files in "$dirname".) }
   return @files;
}

Steps like these make code more readable and make it easy to insert checks.

Community
  • 1
  • 1
amon
  • 57,091
  • 2
  • 89
  • 149
  • Until now i was stuggling with this error and somehow finally able to getrid of it. It is not a solution but instead i have used another method to compare. Thank you all for your support –  Jan 30 '13 at 10:00