views:

226

answers:

4

What is the proper way to write something equivalent to the follow:

while ( my $first = $iterator->next && my $second = $iterator->next ) {
  # do work
}

This does not run - I wanted $first and $second in proper scope inside the while loop.

+1  A: 

Try:

my $first;
my $second;
while (($first = $iterator->next) && ($second = $iterator->next)) {
  # do work
}
Andomar
Why? That is, if `$first` and `$second` are simply loop variables, why give them broader scope?
Telemachus
@Telemachus: Tried `while(my ($a,$b) = (1,1))` then this, but yeah mobrule's answer is better (I voted for it) :)
Andomar
+17  A: 

You want parentheses around the assignment expressions.

while ((my $first = $iterator->next) && (my $second = $iterator->next)) {
   # ...
}

&& has higher precedence than =, so your original code looked like it was trying to do an assignment like x && y = z.

mobrule
+1 Even shorter :)
Andomar
Ah, I thought I was doing it plain wrong, but just a silly mistake. Thanks!
Timmy
+8  A: 

I tend to write something like that as:

 while( my( $first, $second ) = map { ... } 1..2 ) {
      ...
      }

You may not like that syntax because you aren't used to doing things that way, but it follows a couple rules that I think make code easier:

  • I don't type the same thing twice
  • I assign to all the variables at once
  • I use the map to generate the list when I need to run something more than once.
  • I don't use logical operators gratuitously.

However, there's another problem. You have to figure out what you are really testing in the while condition, and make that apparent to the next programmer. I'm not sure why you have two things in that condition. Is it okay to read the iterator if there is only one thing (i.e. what happens to the unprocessed odd man out)?

A much better solution would be some way that you show what you are trying to do without showing the mechanics. Not knowing anything about your iterator, I might suggest that you decorate it with another method that returns two items (or maybe the empty list if it is at the end):

 while( my( $first, $second ) = $iterator->read_two ) {
      ...;
      }

If that doesn't make the situation clear, decorate it with a method to ask a particular question:

 while( $iterator->two_things_left ) {
      my( $first, $second ) = $iterator->read_two;
      ...;
      }
brian d foy
+11  A: 

This is where and would be more appropriate than &&:

#!/usr/bin/perl
use strict; use warnings;

my $it = make_iterator();

while ( my $first = $it->() and my $second = $it->() ) {
    print "$first\t$second\n";
}

sub make_iterator {
    my @x = qw(a b c);
    return sub {
        return shift(@x) if @x;
        return;
    };
}

Output:

C:\Temp> it
a       b

I would be tempted to take the assignment to $second out of the while (depends on the meaning of the iterator not returning an even number of elements):

while ( my $first = $it->() ) {
    defined(my $second = $it->())
        or warn "Iterator returned odd number of elements\n"
        and last;
    print "$first\t$second\n";
}

Output:

C:\Temp> it
a       b
Iterator returned odd number of elements
Sinan Ünür