On Mon, 10 Dec 2001, James S. Martin wrote:

> Hello,
>
> I'm having problems with this subroutine I'm working.  What it is supposed to
> do is find the newest file (the date stamp from the file, collected in a
> different sub) from a list of files with the same name.  My logic is to go
> through the listing of files, and for each item in the array, compare it to
> every item in the array (using the second foreach).  If the name and
> architecture matches, then I want to compare the date strings, find which one
> is newer and remove the old ones out of the array instantly.  What I've
> realized is that splicing elements from the array does actually happen until
> the loop is over, so I'm looping through elements that I don't want to.  Can
> anyone fix my brain/logic?  Thanks,
>
> James
>
>
>
> sub find_newest {
>
> my (@patchlist)=@_;
>
> my $n=0;
> my $x=0;
>
>
> foreach $item(@patchlist) {
>
>
> #print "$item\n";
>
>   ($name1,$date1,$arch1,$file1)=split(/,/,$item);
>
>  foreach $item2(@patchlist) {
>
>     ($name2,$date2,$arch2,$file2)=split(/,/,$item2);
>
>     if ( (($name1.$arch1) eq ($name2.$arch2)) & ($date1 > $date2)) {
>
>
>      print "$name1-$date1 > $name2-$date2.\n";
>      splice(@patchlist,$x,1);
>
>
>     }
>
>     elsif  ( ( ($name1.$arch1) eq ($name2.$arch2) ) & ($date1 < $date2)) {
>
>         print "$name2-$date2 > $name1-$date1.\n";
>         splice(@patchlist,$n,1);
>         }
>
>       $x++;
>
>   }
>
>   $n++;
>
> }
>


Note, there is not much value in comparing an item to itself so
you could add this line to your inner loop:

$x++, next if $x==$n;

Also since when the name.archs are equal you elininate an item unless
the dates are equal so you don't really need two separate if statements
to do that. You could just say:

if (name1.arch1 eq name2.arch2 && $date1 != $date2) {

Of course if you eliminate the $n item then you don't need to compare
the rest of the items to it. Sigh...

But keep in mind that when you eliminate an entry by splicing it
out, you've altered the number of elements in @patchlist so incrementing
over the element won't work. I suggest you maintain a separate hash
of elements to eliminate like:

if ( (($name1.$arch1) eq ($name2.$arch2)) && ($date2 < $date1)) {
    $cutlist{$x}) = 1;
}

then after the loops are done, copy elements from @patchlist unless there
is a corresponding %cutlist entry, e.g.

$x=0;
foreach $element (@patchlist) {
   push(@newlist,$element) unless $cutlist{$x};
   $x++;
}
@patchlist=@newlist;

As a substantal enhancement to the speed note that if
you compare element 2 to element 3 then there is not much
value in comparing element 3 to element 2. Especially if
one or both of them have been eliminated. I would recode the
two loops like:


OUTER:
for ($n=0;$n<=$#patchlist-1;$n++) {
   next if $cutlist{$n};
   ($name1,$arch1,$date1,$file1)=split(/,/,$patchlist[$n]);
   for ($x=$n+1,$x<=$#patchlist,$x++) {
       next if $cutlist{$x};
       ($name2,$arch2,$date2,$file2)=split(/,/,$patchlist[$x]);
       next unless $name1 eq $name2;
       next unless $arch1 eq $arch2;
       if ($date2 < $date1) {
          $cutlist{$x}=1;
          next;
       }
       if ($date1 < $date2) {
          $cutlist($n}=1;
          next OUTER;
       }
  }
}

$x=0;
foreach $element (@patchlist) {
   push(@newlist,$element) unless $cutlist{$x};
   $x++;
}
@patchlist=@newlist;


**** [EMAIL PROTECTED] <Carl Jolley>
**** All opinions are my own and not necessarily those of my employer ****

_______________________________________________
Perl-Win32-Users mailing list
[EMAIL PROTECTED]
http://listserv.ActiveState.com/mailman/listinfo/perl-win32-users

Reply via email to