http://qs321.pair.com?node_id=989537

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

hi guys, I was wondering if someone could help me, I've got a list of strings that need to be scrubbed, my method below isn't the most efficient but i am unsure of other methods that can help me:
if ($colr =~'baby blue') { $trimmedcolr = 'BB'; } if ($colr =~'baby pink') { $trimmedcolr = 'BP'; } if ($colr =~'dark blue') { $trimmedcolr = 'DB'; } if ($colr =~'dark purple') { $trimmedcolr = 'DP'; } if ($colr =~'hot pink') { $trimmedcolr = 'HP'; } if ($colr =~'light purple') { $trimmedcolr = 'LP'; } else { ($trimmedcolr = $colr) }
whilst the code is run it only replaced the strings that contain light pink excluding everything else? please can someone help me fix this? Thank you :)

Replies are listed 'Best First'.
Re: If statement not working
by toolic (Bishop) on Aug 24, 2012 at 13:53 UTC
    You need to change all but the 1st if to elsif:
    if ( $colr =~ 'baby blue' ) { $trimmedcolr = 'BB'; } elsif ( $colr =~ 'baby pink' ) { $trimmedcolr = 'BP'; } elsif ( $colr =~ 'dark blue' ) { $trimmedcolr = 'DB'; } elsif ( $colr =~ 'dark purple' ) { $trimmedcolr = 'DP'; } elsif ( $colr =~ 'hot pink' ) { $trimmedcolr = 'HP'; } elsif ( $colr =~ 'light purple' ) { $trimmedcolr = 'LP'; } else { ( $trimmedcolr = $colr ); }

    See also:

Re: If statement not working
by cheekuperl (Monk) on Aug 24, 2012 at 13:59 UTC
    If you have a hash like:
    %mapping=( 'baby blue'=>'BB', 'baby pink'=>'BP', 'dark blue'=>'DB', #and so on... );
    Wouldn't that reduce your if-else series to this:
    $trimmedcolr=$mapping{$colr};
    Also, if new colors are added, you'd only need to add an entry in the hash.
Re: If statement not working
by Marshall (Canon) on Aug 24, 2012 at 13:58 UTC
    I don't see any "light pink" color. The code is not efficient, but it is not horrific. Your question appears to be about the logic of the statements rather than the efficiency.

    Under normal UI processing, the use of "elsif" would make no practical difference. Please further clarify what you desire.

    Update: show some data and explain clearly "why it is not working". Use of "elsif" or hash tables don't seem to go to the point. Please explain why these simple "if" statements are not working for you.

      Use of "elsif" or hash tables don't seem to go to the point.

      Actually either of those approaches does solve the problem, which is (if I'm understanding the OP correctly) that:

      The OP thinks that the 'else' clause will be executed if NONE of the 'if' statements match, whereas actually the 'else' clause will be executed whenever the FINAL 'if' statement fails to match.

      i.e. OP's intention was

      if () {} elsif () {} elsif () {} else {}
      but OP has got the syntax wrong and actually coded
      if (a) {}; if (b) {}; if (c) {}; if (d) {} else {default()};
      where even if one of a, b or c is true then default() is still executed (and overwrites the variable) so long as d is false.

      IIU the OP correctly.

        I see what you mean, that this is a "dangling else" clause.

        I guess something like:

        $trimmedcolr = $colr; $trimmedcolr = 'BB' if $colr eq 'baby blue'; $trimmedcolr = 'BP' if $colr eq 'baby pink'; $trimmedcolr = 'DB' if $colr eq 'dark blue'; $trimmedcolr = 'DP' if $colr eq 'dark purple'; $trimmedcolr = 'HP' if $colr eq 'hot pink'; $trimmedcolr = 'LP' if $colr eq 'light purple';
        would have worked although the hash based solutions in this thread are probably more efficient. BTW, I have no problem with this trailing "if" syntax in a situation like this were the intent and readability is very clear.

        The "why" question wasn't asked. However since Perl is so good at processing strings, I would suggest that translating an easily understandable string into a shorter more cryptic string is usually just not necessary or advisable.

        I would not do this translation without a good reason. One reason might be that this program talks to something else that only understands the two letter abbreviations, however in that case, the default of not abbreviating the color doesn't appear to make sense. Making this a subroutine and returning an error in the case of an unknown color might make sense. I don't know what this "abbreviate it if you can" idea accomplishes.

        Update:
        Without a good reason, I wouldn't do this, but consider this:

        #!/usr/bin/perl -w use strict; my @colors = ( 'baby blue', 'baby pink', 'dark blue', 'dark purple', 'hot pink', 'light purple', 'wild green', 'crazy yellow', 'wild crazy purple', 'deep purple'); foreach (@colors) { print getColorAbreviation($_),"\n"; } # The translation algorithm appears to be straight- # forward, so a table independent translation is # possible ... Of course 'dark purple' and 'deep purple' # would translate into the same thing, but maybe that is # ok? This is application dependent. sub getColorAbreviation { my $color = shift; #like: 'wild crazy purple' $color = uc $color; my @FirstCaps = $color =~ /(\w)\w+/g; return @FirstCaps; #like: WCP } __END__ BB BP DB DP HP LP WG CY WCP DP
Re: If statement not working
by andye (Curate) on Aug 24, 2012 at 14:54 UTC
    It's because of your final statement, here:

    if ($colr =~'light purple') { $trimmedcolr = 'LP'; } else { $trimmedcolr = $colr; }

    (I've tidied it up a bit to make it clearer).

    What you're saying with that is:

    IF it's "light pink" then set trimmedcolr to 'LP' ELSE (if it's not "light pink") then set trimmedcolr to colr.
    So, if any of the statements before that final one match, then 'light pink' won't match and so the ELSE part of that final statement will be executed.

    e.g. if the colour is 'hot pink' then $trimmedcolr will be set to 'HP' and then set right back to $colr (in the bottom line).

    HTH!

Re: If statement not working
by GrandFather (Saint) on Aug 25, 2012 at 01:05 UTC

    I'd use a hash to manage the colour/replacement pairs. Consider:

    use strict; use warnings; my %colours = ( 'baby blue' => 'BB', 'baby pink' => 'BP', 'dark blue' => 'DB', 'dark purple' => 'DP', 'hot pink' => 'HP', 'light purple' => 'LP', 'pink' => 'PK', 'pinkish' => 'PI', ); my $colourMatch = join '|', sort {length $b <=> length $a || $a cmp $b} keys %colours; for my $colour ('baby blue', 'baby pink', 'baby green', 'pink', 'pinki +sh') { if ($colour =~ /($colourMatch)/) { print "Matched $colour: $colours{$1}\n"; } else { print "Unmatched: $colour\n"; } }

    Prints:

    Matched baby blue: BB Matched baby pink: BP Unmatched: baby green Matched pink: PK Matched pinkish: PI

    Note that the sort is there so that pinkish is matched correctly rather then being matched by 'pink'. Depending on the order of your tests and the strings being tested you current code would suffer from the same problem.

    True laziness is hard work
Re: If statement not working
by NetWallah (Canon) on Aug 24, 2012 at 15:14 UTC
    • Setup a Hash like this: my %shortname=( 'baby blue'=>'BB, 'dark purple'=>DB ...etc);
    • for my $c (keys %shortname){$colr =~s/($c)/$shortname{$c}/g ;}

                 I hope life isn't a big joke, because I don't get it.
                       -SNL