Re: Re: Searching a database

I've been doing Perl for about 5 years, and I'm *NO* expert, but I have to say that to me that is at least somewhat obfuscated code. I have no problem with the while, but the foreach:

    foreach my $num (split /::/ => $value) {
[download]

mystifies me - what is the split splitting? When split isn't given an arg to split, it splits $_, but what is $_ in this case, since the while loop gets $key and $value?

IMHO, more straightforward code, albeit sometimes longer code, is better:

  my %matches = map {$_ => 1} 17, 83, 114, 205, 407;  
  while (my ($key, $value) = each %hash) {
    my @value_tokens = split /::/, $value;
    foreach my $value_token (@value_tokens) {
      if ($matches{$value_token}) {
         print "$key: $value\n";
         $found = 1;
         last; ### break out of foreach
      }
    } ### end foreach
    if $found:
      next;
    }
  } ### end while
[download]

I'm not entirely sure I understand the original intent, so this solution may need some work.

HTH.

Comment on Re: Re: Searching a database Select or Download Code

Replies are listed 'Best First'.
Re: Searching a database by Abigail-II (Bishop) on Sep 12, 2003 at 13:35 UTC
I have no problem with the while, but the foreach: `foreach my $num (split /::/ => $value) {` [download] mystifies me - what is the split splitting? When split isn't given an arg to split, it splits $_ split() splits its second argument, if any. In the quoted line, the second argument of split() is $value. MHO, more straightforward code, albeit sometimes longer code, is better: I spot two differences. First one is that you replace the split inside the foreach into two lines, introducing an extra variable: `my @value_tokens = split /::/, $value; foreach my $value_token (@value_tokens) {` [download] I'm not convinced of any gain here. The second one is that you replace a labelled `last` with a Pascal/Cish equivalent: setting a flag in the inner loop, and testing for it in the outer loop. This is what the documentation (and I hear Larry's voice when I read it) has to say about it: Here's how a C programmer might code up a particular algo- rithm in Perl: rithm in Perl: for (my $i = 0; $i < @ary1; $i++) { for (my $j = 0; $j < @ary2; $j++) { if ($ary1[$i] > $ary2[$j]) { last; # can't go to outer :-( } $ary1[$i] += $ary2[$j]; } # this is where that last takes me } Whereas here's how a Perl programmer more comfortable with the idiom might do it: OUTER: for my $wid (@ary1) { INNER: for my $jet (@ary2) { next OUTER if $wid > $jet; $wid += $jet; } } See how much easier this is? It's cleaner, safer, and faster. It's cleaner because it's less noisy. It's safer because if code gets added between the inner and outer loops later on, the new code won't be accidentally exe- cuted. The "next" explicitly iterates the other loop rather than merely terminating the inner one. And it's + faster because Perl executes a "foreach" statement more rapidly than it would the equivalent "for" loop. [download] The documentation calls the use of labelled loop control cleaner, safer, faster. And you prove the documentation right. Your `if $found: next; }` [download] would, after translating the Python syntax to Perl syntax, contributes nothing at all. It's at the end of the loop and doing the next iteration is what's going to happen anyway. It means that your code is going to inspect all key-value pairs of the hash, instead of bailing out when a matching value is found. Bailing out early is more efficient, and correct, because it's given that there is at most one matching value. It's not that my algorithm is flawless, it does contain something to pick a nit over: it leaves the hash iterator as is. But that could be easily resolved by doing an 'keys %hash' after the while loop. Abigail	[reply] [d/l] [select]

Replies are listed 'Best First'.

Re: Searching a database
by Abigail-II (Bishop) on Sep 12, 2003 at 13:35 UTC

I have no problem with the while, but the foreach:
foreach my $num (split /::/ => $value) {
[download]
mystifies me - what is the split splitting? When split isn't given an arg to split, it splits $_

split() splits its second argument, if any. In the quoted line, the second argument of split() is $value.

MHO, more straightforward code, albeit sometimes longer code, is better:

I spot two differences. First one is that you replace the split inside the foreach into two lines, introducing an extra variable:

my @value_tokens = split /::/, $value;
foreach my $value_token (@value_tokens) {
[download]

I'm not convinced of any gain here.

The second one is that you replace a labelled last with a Pascal/Cish equivalent: setting a flag in the inner loop, and testing for it in the outer loop.

This is what the documentation (and I hear Larry's voice when I read it) has to say about it:

       Here's how a C programmer might code up a particular algo-
       rithm in Perl:
       rithm in Perl:

           for (my $i = 0; $i < @ary1; $i++) { 
               for (my $j = 0; $j < @ary2; $j++) {
                   if ($ary1[$i] > $ary2[$j]) {
                       last; # can't go to outer :-(
                   }
                   $ary1[$i] += $ary2[$j];
               }
               # this is where that last takes me
           }

       Whereas here's how a Perl programmer more comfortable with
       the idiom might do it:
   
           OUTER: for my $wid (@ary1) {
           INNER:   for my $jet (@ary2) {
                       next OUTER if $wid > $jet;
                       $wid += $jet;  
                    }
                 }  
   
       See how much easier this is?  It's cleaner, safer, and
       faster.  It's cleaner because it's less noisy.  It's safer
       because if code gets added between the inner and outer
       loops later on, the new code won't be accidentally exe-
       cuted.  The "next" explicitly iterates the other loop
       rather than merely terminating the inner one.  And it's        
+          
       faster because Perl executes a "foreach" statement more
       rapidly than it would the equivalent "for" loop.
[download]

The documentation calls the use of labelled loop control cleaner, safer, faster. And you prove the documentation right. Your

    if $found:
        next;
    }
[download]

It's not that my algorithm is flawless, it does contain something to pick a nit over: it leaves the hash iterator as is. But that could be easily resolved by doing an 'keys %hash' after the while loop.

Abigail

[reply]
[d/l]
[select]