Different result for 'foreach' vs 'while shift' arrayref

[gvandeweyer]

Dear Monks,
I having problems understand why the following two snippets give different result (the second one being correct):
Situation: Arrayref with database rows, returned from subroutine, in my test-case 927306 rows:

my $rowcache = runQuery($query,"Table",\@ids);
print "RunQuery results: ".scalar(@$rowcache)." rows\n";

Snippet one: Using while shift to return rows yields only 788425 resulting rows

while (my $result = shift(@$rowcache) ) {   
   print OUT $result->{'id'}."\t".$result->{'Name'}."\t".$result->{'Co
+de'}."\t".$result->{'Date'}."\t".$result->{'Comment'}."\n";
}

Snippet two : Using foreach returns all rows:

foreach my $result (@$rowcache)  {
    print OUT $result->{'id'}."\t".$result->{'Name'}."\t".$result->{'C
+ode'}."\t".$result->{'Date'}."\t".$result->{'Comment'}."\n";
}

I thought that these should be equivalent?
Best,
Geert

[tobyink]

With this one:

while (my $result = shift(@$rowcache) )

If @$rowcache contains a false value (e.g. undef, or the empty string, or the number 0), then when that value is reached, the condition will become false, so the while loop will exit.

If you really want to use while, then you could do:

while (my ($result) = @$rowcache ? shift(@$rowcache) : ())

Or, if you know that @$rowcache consists of only defined values (no undefs, but perhaps some empty strings and zeros), you could do:

while ( defined(my $result = shift @$rowcache) )

[Your Mother]

And I think this, while breaking the mental encapsulation of the single line versions, might be easier for some hackers to grok. No personal preference here; just tossing one more on the pile.

while ( @{$rowcache} ) {
    my $result = shift @{$rowcache};
    ....

[tobyink]

That's certainly true. I often use that, and have no idea why I didn't suggest it myself!

Generally speaking the reason to use this idiom is when you need to loop through a list and in each iteration you need to take one or more items from the list.This kind of thing:

use strict;
use warnings;
use Data::Dumper;

my @input = (
    'foo:'   => 1,
    'bar:'   => 2,
    'baz',  # no colon, so value is "1"
    'quux:'  => 3,
);

my %output;
while (@input)
{
    my $key = shift @input;
    
    if ($key =~ /\A(.+):\z/)
    {
        $output{$1} = shift @input;
    }
    else
    {
        $output{$key} = 1;
    }
}

print Dumper \%output;

If you only need to step through the list one at a time, use foreach; it's clearer.

[Not_a_Number]

They are not equivalent.

Something in your arrayref (namely the 788426th item) evaluates to 0 or another false value, so your while loop ends there. Consider

my @ary = ( 1, 6, 0, 2, 3 );
while ( my $result = shift( @ary ) ) {
  print "$result\n";
}

Output:

1
6

[rnewsham]

Hi Geert,

Without seeing the full code and data it is hard to say what is happening. I would recommend trying to find which rows are missing from your while iterations. If they are rows that contain a specific value, they are the first rows or last rows that may give you the clue you need. I would also check that you do not have two loops over $rowcache which use shift, as an earlier shift could be removing the data.

Richard

[gvandeweyer]

It turned out that there were a few undef elements in the rowcache. Thank you for the pointers!
geert

[AnomalousMonk]

... there were a few undef elements in the rowcache.

Use of warnings would have enabled Perl to alert you to this situation, and
    use warnings FATAL => 'all';
would have put a quick and merciful end to it.

c:\@Work\Perl>perl -wMstrict -le
"use warnings FATAL => 'all';
 ;;
 my $hr = { foo => 'bar' };
 my @ra = ($hr, $hr, undef, $hr, $hr);
 ;;
 for my $hashref (@ra) {
   print qq{foo is '$hashref->{foo}'};
   }
"
foo is 'bar'
foo is 'bar'
Use of uninitialized value in concatenation (.) or string at -e line 1
+.

[Anonymous Monk]

Hi :)

Hmm, I've never done that :) when I see those statements i think: I'd write while array not empty ... modify array so

Read more... (1250 Bytes)

So, very nice thread/question gvandeweyer, makes made me examine why I write code the way I write ... its amazing how much stuff just slips into your mind while you're unaware ... I believe I learned to write while(@array) from good code examples ... after its the way I think :)

Hmm, I'm getting another idea, if you didn't notice your array values can be undef while testing/developing, a supplemental warning could be useful; should be easy to write since Perl::Critic::Policy is xpaths , and naming is always the hard part
Perl::Critic::Policy::BuiltinFunctions::ProhibitWhileShift - don't write while($foo=shift@bar) instead write while(@bar)
Perl::Critic::Policy::ControlStructures::ProhibitWhileShiftArray - don't write while($foo=shift@bar) instead write while(@bar)
Perl::Critic::Policy::Variables::ProhibitWhileLoopShiftArray - don't write while($foo=shift@bar) instead write while(@bar)
Perl::Critic::Policy::ControlStructures::LoopTestConditionArray - don't write while($foo=shift@bar) instead write while(@bar)

Thanks again gvandeweyer

[Anonymous Monk]

The module site\lib\Perl\Critic\Policy\ControlStructures\ProhibitShiftLoopCondition.pm

Read more... (5 kB)

the "test" file

Read more... (1368 Bytes)

[sundialsvc4]

Incidentally, is there documentation of the behavior of foreach if the contents of the array are volatile during the loop?   And of unshift vs. push in such a frightful but-it-happens scenario?

[kcott]

Yes. See the "Foreach Loops" section of "perlsyn - Perl syntax":

"If any part of LIST is an array, foreach will get very confused if you add or remove elements within the loop body, for example with splice. So don't do that."