No Pause on Elsif in Debugger, Part 2

QM has asked for the wisdom of the Perl Monks concerning the following question:

As another example of unhelpful behavior (similar to No Pause on Elsif in Debugger), here's some code I was modifying today:

###############################
# try to sort fields as numbers first, then as strings
# keep going until there's a difference

sub smart_sort
{
  my @a = split /[\s_]+/, lc($a);
  my @b = split /[\s_]+/, lc($b);
  
  for my $i ( 0..(@a < @b ? $#a : $#b) )
  {
    if ( $a[$i] =~ $NUMBER_ONLY_REGEX  # line 214
     and $b[$i] =~ $NUMBER_ONLY_REGEX
     and $a[$i] <=> $b[$i] )
    {
      return $a[$i] <=> $b[$i];
    }
    elsif ( ( my @ai = /^(\d+)(.+)/ )  # error/bug is here
        and ( my @bi = /^(\d+)(.+)/ ) )
    {
      if ( $ai[0] <=> $bi[0] )
      {
        return $ai[0] <=> $bi[0];
      }
      elsif ( $ai[1] cmp $bi[1] )
      {
        return $ai[1] cmp $bi[1];
      }
    }  
    elsif( $a[$i] cmp $b[$i] )
    {
      return $a[$i] cmp $b[$i];
    }
  }
  return $a cmp $b;
}
[download]

gives the error:

Use of uninitialized value in pattern match (m//) at /user/sgriffit/perl/mergen.pl line 214.

Line 214 is only the start of the if/elsif/... chain. Not too helpful. Glad there's only 1 place where a match is happening.

Now I realize there are several things there that could be changed to improve the error reporting. For instance, since most of the blocks return, the elsifs could be ifs instead.

I don't really want to spend time thinking "If this code has a bug, what's the best way of writing it so it's easy to spot?" Perhaps I'm flogging a dead horse...

Here's the cleaned up version, with more comments:

sub smart_sort
{
  my @a = lc($a) =~ /(\d+|[a-z]+)/ig; # better split on fields
  my @b = lc($b) =~ /(\d+|[a-z]+)/ig;

  for my $i ( 0..(@a < @b ? $#a : $#b) )
  {
    # try numbers first
    if ( $a[$i] =~ $NUMBER_ONLY_REGEX 
     and $b[$i] =~ $NUMBER_ONLY_REGEX
     and $a[$i] <=> $b[$i] )
    {
      return $a[$i] <=> $b[$i];
    }
    
    # fallback to string compare
    if( $a[$i] cmp $b[$i] )        # no "elsif"
    {
      return $a[$i] cmp $b[$i];
    }
  }
  
  # can't decide by fields, decide by whole string
  return $a cmp $b;
}
[download]

-QM
--
Quantum Mechanics: The dreams stuff is made of

Comment on No Pause on Elsif in Debugger, Part 2 Select or Download Code

Replies are listed 'Best First'.
Re: No Pause on Elsif in Debugger, Part 2 by graff (Chancellor) on Sep 25, 2006 at 01:35 UTC
I don't really want to spend time thinking "If this code has a bug, what's the best way of writing it so it's easy to spot?" One the one hand, I'd say "well then don't spend time thinking about that", but OTOH, I don't actually see anything wrong with thinking that way. If the debugger is a tool you use regularly, there's no avoiding the fact that you will become familiar with its limitations, and a natural outcome will be to adjust the coding style to avoid them. To wit: I realize there are several things there that could be changed to improve the error reporting. For instance, since most of the blocks return, the elsifs could be ifs instead. If you went ahead and did that, would your code be worse in any way than it is now? Not really. In fact, this particular example would seem nice if it were phrased like this: `for my $i ( 0..(@a < @b ? $#a : $#b) ) { return $a[$i] <=> $b[$i] if ( $a[$i] =~ $NUMBER_ONLY_REGEX and $b[$i] =~ $NUMBER_ONLY_REGEX and $a[$i] != $b[$i] ); return $a[$i] cmp $b[$i] if ( $a[$i] ne $b[$i] ); } return $a cmp $b;` [download] I do agree that the issues with compiler error reporting, as discussed in your other thread, can be a problem. If I felt like I had to develop a coding style specifically to overcome this limitation, I might try something like this: `if ( condition1() ) { do_first_thing(); goto ENDIF_1; } if ( $undeclared_scalar = condition2() ) { do_second_thing(); do_something_else(); goto ENDIF_1; } { do_default_thing(); $other_default_work = even_with_typos; } ENDIF_1: # and now back to our regular programming...` [download] That makes it easy/possible for "perl -c" to report meaningful line numbers for typos, and allows me to pause the debugger at each condition. Once I'm past that stage and it's working okay, I could go over it one last time and easily change it to "if ... elsif ... else", remove or comment out the "goto" statements and the "ENDIF_1" label, and everything's back to normal. Even the line numbering stays the same. (I doubt that I would do this globally or by habit, but I could easily imagine using it as a fallback strategy on an as-needed basis, just to get some honest ground truth from the compiler and debugger when I'm not able to spot the bug easily on my own.)	[reply] [d/l] [select]

Replies are listed 'Best First'.

Re: No Pause on Elsif in Debugger, Part 2
by graff (Chancellor) on Sep 25, 2006 at 01:35 UTC

I don't really want to spend time thinking "If this code has a bug, what's the best way of writing it so it's easy to spot?"

One the one hand, I'd say "well then don't spend time thinking about that", but OTOH, I don't actually see anything wrong with thinking that way. If the debugger is a tool you use regularly, there's no avoiding the fact that you will become familiar with its limitations, and a natural outcome will be to adjust the coding style to avoid them. To wit:

I realize there are several things there that could be changed to improve the error reporting. For instance, since most of the blocks return, the elsifs could be ifs instead.

If you went ahead and did that, would your code be worse in any way than it is now? Not really. In fact, this particular example would seem nice if it were phrased like this:

  for my $i ( 0..(@a < @b ? $#a : $#b) ) {
    return $a[$i] <=> $b[$i]
        if ( $a[$i] =~ $NUMBER_ONLY_REGEX and 
             $b[$i] =~ $NUMBER_ONLY_REGEX and
             $a[$i] != $b[$i] );
    return $a[$i] cmp $b[$i]
        if ( $a[$i] ne $b[$i] );
  }
  return $a cmp $b;
[download]

your other thread

if ( condition1() ) {
    do_first_thing();     goto ENDIF_1;
}
if ( $undeclared_scalar = condition2() ) {
    do_second_thing();
    do_something_else();  goto ENDIF_1;
}
{
    do_default_thing();
    $other_default_work = even_with_typos;
}
ENDIF_1:  # and now back to our regular programming...
[download]

(I doubt that I would do this globally or by habit, but I could easily imagine using it as a fallback strategy on an as-needed basis, just to get some honest ground truth from the compiler and debugger when I'm not able to spot the bug easily on my own.)

[reply]
[d/l]
[select]