Re: wxPerl Simulated 7 Segment LCD Display

[Athanasius]

in reply to wxPerl Simulated 7 Segment LCD Display

As ww says, nicely done! A couple of minor points:

1. In sub DrawSegment{, the line:

   elsif($segment = 6) {                    # Draw the 6 sided segment(6)
   

   is almost certainly a mistake: it sets $segment to the value 6, which is “true”, so the following else block will never be reached. Replace = with ==.

2. A subroutine such as:

   sub GetValue{
       my $return = $wxGlobals{mValue};
   }
   

   would be better written as:

   sub GetValue{
       return $wxGlobals{mValue};
   }
   

   as the lexical variable $return serves no purpose here — it is being created and initialised only to be immediately thrown away. Likewise, this:

   sub Decode {                            # Table lookup for character t
   +o
       my($char) = @_;                     # Segment translation
       my $return;
       if(defined($ctbl{$char})) {
           $return = $ctbl{$char};
       }
       else {
            $return = $ctbl{'='};          # Triple bar for undefined cha
   +racter
       }   
   }
   

   works, but only (in a sense) by accident: the last expression evaluated will be an assignment to $return, so the value assigned will be returned by the sub. But with a small code change, this logic could easily break. Simpler, safer, and clearer:

   sub Decode                              # Table lookup for character t
   +o segment translation
   {
       my ($char) = @_;
   
       if (defined $ctbl{$char})
       {
           return  $ctbl{$char};
       }
   
       return $ctbl{'='};                  # Triple bar for undefined cha
   +racter
   }
   

   Update: ++BrowserUk for the much better version below!

Hope that helps,

[BrowserUk]

Simpler, safer, and clearer:

Or just:

# Table lookup for character to segment translation
sub Decode { $ctbl{ $_[0] } // $ctbl{ '=' } }