Beefy Boxes and Bandwidth Generously Provided by pair Networks
Welcome to the Monastery
 
PerlMonks  

Replacing If Elsif Else with Hash

by johnfl68 (Beadle)
on Nov 24, 2012 at 04:59 UTC ( #1005331=perlquestion: print w/ replies, xml ) Need Help??
johnfl68 has asked for the wisdom of the Perl Monks concerning the following question:

Hello:

I had the following if elsif else:

if ($severity eq "Extreme"){ $background = "red"; } elsif ($severity eq "Severe"){ $background = "orange"; } elsif ($severity eq "Moderate"){ $background = "yelow"; } elsif ($severity eq "Minor"){ $background = "green"; } else { $background = "grey"; }
It worked fine in the script as it was, but when I placed that script into a external subroutine, it broke things (took me a while to figure out that was the part of code causing the issue). So in reading around, many suggest using a hash instead of long if elsif sets. I understand that, and so now I have this:
my %bkcolor = ( "Extreme", "red", "Severe", "orange", "Moderate", "yellow", "Minor", "green", "", "grey" ); my $background = $bkcolor{$severity};

The question I have is, is there a way to handle the last 'else' when doing things in this manor, so that if 'severity' is anything other than the 4 known responses, it will always be grey?

Or is there another way I should be doing this?

Thanks as always!

John

Comment on Replacing If Elsif Else with Hash
Select or Download Code
Re: Replacing If Elsif Else with Hash
by frozenwithjoy (Curate) on Nov 24, 2012 at 05:07 UTC
    This will return 'grey' if you use a severity not defined in the hash:
    my %bkcolor = ( 'Extreme' => 'red', 'Severe' => 'orange', 'Moderate' => 'yellow', 'Minor' => 'green', ); my $background = $bkcolor{$severity} // 'grey';

      Thank you very much for you time and help!

      John

Re: Replacing If Elsif Else with Hash
by AnomalousMonk (Monsignor) on Nov 24, 2012 at 07:59 UTC
    It worked fine in the script as it was, but when I placed that script into a external subroutine, it broke things...

    I endorse your desire to use a dispatch table of some kind rather than the original approach, but I am curious to know how things 'broke' when you copied the  if ... elsif ... else structure to other code. Can you elaborate?

      To be honest, I am not exactly sure. The portion of code involves some simple Image::Magick and would run, but would not write the final output image from the external subroutine. I spent most of the day trying to figure out why the image wasn't being created.

      I ended up slowly rebuilding the subroutine with small portions, and running with each small addition until the point when the image wasn't being written again, and it turned out to be the if elsif else portion. In the end, I had a subroutine with exactly the same code, with everything except that section.

      I use strict and warnings, nothing ever showed up, so I can only assume there was some sort of collision with Image::Magick but not sure what. I even tried changing both $severity and $background to different string names, at first thinking maybe one of them was colliding with something, but still, no image would be written.

      With both the hash version I did, and the one suggested by frozenwithjoy, they both work fine in the external sub.

      I am still curious as to why it did not work that way, but in the end I am happy that I was at least able to track down what part of the code was causing the problem, and find a solution.

        johnfl68:

        If everything worked find (i.e. no error messages, etc.) and if failed only because of the addition of the if/else block, then I think I'd suspect one or more of your braces was a line too high or too low, causing the code to be bypassed or some such. Pure speculation, though.

        ...roboticus

        When your only tool is a hammer, all problems look like your thumb.

Re: Replacing If Elsif Else with Hash
by TomDLux (Vicar) on Nov 24, 2012 at 16:33 UTC

    I would suggest putting this chunk into a routine. It provides some documentation, it becomes easier to modify behaviour in the future, because any changes are isolated within a short routine, and implementation details are separated from business logic.

    use Readonly; Readonly my %BGCOLOR => ( Extreme => 'red', ... ); Readonly my $DEFAULT_BGCOLOR => 'grey'; sub get_bg_color { my ( $severity ) = @_; return $DEFAULT_BGCOLOR unless exists $BGCOLOR{$severity}; return$BGCOLOR{$severity}; } my $bg = get_bg_color( $condition );

    edit - corrected typo $BGCOLOR => %BGCOLOR; thanks, Anon Monk!

    As Occam said: Entia non sunt multiplicanda praeter necessitatem.

      I strongly agree with this ... no matter how exactly you choose to write the routine or to initialize the hash.

      When writing this sort of logic, be extremely careful to test for the existence, or the absence, of the hash key ... not for “falsehood.”   Someday you will have a hash-entry whose legitimate value translates to false.   Or even maybe to undef.   It’s just a hungry sort of “gotcha” that’s just waiting for your glutes.

Log In?
Username:
Password:

What's my password?
Create A New User
Node Status?
node history
Node Type: perlquestion [id://1005331]
Approved by frozenwithjoy
Front-paged by 2teez
help
Chatterbox?
and the web crawler heard nothing...

How do I use this? | Other CB clients
Other Users?
Others contemplating the Monastery: (10)
As of 2014-08-20 20:43 GMT
Sections?
Information?
Find Nodes?
Leftovers?
    Voting Booth?

    The best computer themed movie is:











    Results (124 votes), past polls