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

Hi,

Is it possible to check two variables in given/when statement in below code? ( ie something like given $month , $week ) How to simplify the nested if conditions in below code ? Just thinking to have one subroutine to have few parameters from second for loop to avoid nested ifs & elsifs or any other suggestion is appreciated

update: How to get exit codes for different checks ie month/week base and print successful value , failure value and exit 1 for any one of failure case in if condition for value comparision?

Sample data & snippet code is mentioned below: 

 
#!/usr/bin/perl
use warnings;
use strict;
use feature "switch";

my %Hash = ();
$Hash{"FEB"}{"W1"}=10.14;
$Hash{"FEB"}{"W2"}=11.22;
$Hash{"MAR"}{"W1"}=33.17;
$Hash{"MAR"}{"W2"}=44.44;
$Hash{"APR"}{"W1"}=55.00;
$Hash{"APR"}{"W2"}=66.66;

my $FEB_L1_W1 = 11;
my $FEB_U1_W2 = 13;
my $APR_L1_W1 = 1;
my $APR_U1_W2 = 13.02;


for my $month ( keys %Hash ) {
        print "$month: ";
        for my $week ( keys %{ $Hash{$month} } ) {
        print "$week=$Hash{$month}{$week} \n";
        given ($month) {
        
        when ('FEB') {
        if ( $week eq "W1" )  {
        if ( ( $FEB_L1_W1 <= $Hash{$month}{$week}  ) and ( $Hash{$mont
+h}{$week} <= $FEB_U1_W2 ) ) {
            print "month $month has f status for  $week: $Hash{$month}
+{$week}\n";
                }
                else {
                    print "error: print something";
                }
        } elsif ( $week eq "W2" )  {
                if (    ( $FEB_U1_W2 <= $Hash{$month}{$week}  )
                    and ( $Hash{$month}{$week} <= $FEB_U1_W2 ) )
                {
                    print "month $month has f status for  $week: $Hash
+{$month}{$week}\n";
                }
                else {
                    print "error: print something\n";
                }
        }
        }
        when ('APR') { 
        # same as above check for lower , upper values 
        print "it is $month here\n";}
        }
    }
}

[download]

Replies are listed 'Best First'.
Re: Simplify HoH code
by haukex (Archbishop) on Dec 03, 2017 at 10:08 UTC

    Note that given/when and smart matching is considered experimental, and might change in future releases of Perl (this is in fact currently being discussed). In this case, your conditions are so simple that they could be replaced by a simple if/elsif. That's also how I would suggest solving your question about matching multiple variables at once, e.g. if ($month eq 'FEB' && $week eq 'W1').

    Also, one thing to "simplify" the code would be to use consistent formatting (like where you put the braces) and indentation. perltidy could help you with this.

    Lastly, is this code really representative of your actual code? The reason I ask is that you have variables named like $FEB_L1_W1. I see two possibilities: Either you have a fixed set of variables for specific months that won't change. In that case, you don't need to loop over the keys of %Hash to find those months - just say $Hash{FEB} and $Hash{APR} to access those.

    Or, you are planning your actual code to have a bunch more variables named $MONTH_L1_W1 etc. In that case, you shouldn't store them as individual variables, but instead be putting those in a hash as well, in which case you can get rid of the repetitive code in the loop and loop over both hashes. We could probably show an example of this, but at the moment I don't quite understand what your code is doing (probably because you haven't explained the magic numbers in your code).

    If you could explain more about what your code is supposed to be doing, and show an SSCCE that is perhaps more representative, and some more sample input and the expected output, then we could probably help more and suggest better solutions.

[reply]
[d/l]
[select]
      Firs thanks for quick reply. Yes , initially i had if else instead of given/when . I switched to given/when later maybe bad choice :( i will use perltidy for formatting. Actual content will be in that format. Ok to have hash there too insteadof bunch of variables but considering HoH loops & nested ifs, Not sure to check with this new hash against HoH second loop. i added updated script with few more variables and if checks and comment on exit/print information from perltidy output: 
      #!/usr/bin/perl
use warnings;
use strict;
use feature "switch";

my %Hash = ();
$Hash{"FEB"}{"W1"} = 10.14;
$Hash{"FEB"}{"W2"} = 11.22;
$Hash{"MAR"}{"W1"} = 33.17;
$Hash{"MAR"}{"W2"} = 44.44;
$Hash{"APR"}{"W1"} = 55.00;
$Hash{"APR"}{"W2"} = 66.66;

my $FEB_L1_W1 = 11;
my $FEB_U1_W1 = 12.05;
my $FEB_L1_W2 = 13.06;
my $FEB_U1_W2 = 13.06;
my $APR_L1_W1 = 1;
my $APR_U1_W1 = 20;
my $APR_L1_W2 = 33;
my $APR_U1_W2 = 13.02;
my $MAR_L1_W1 = -12.05;
my $MAR_U1_W1 = -14.0;
my $MAR_L1_W2 = 22.0;
my $MAR_U1_W2 = -13.02;

for my $month ( keys %Hash ) {
    print "$month: ";
    for my $week ( keys %{ $Hash{$month} } ) {
        print "$week=$Hash{$month}{$week} \n";

        # Did you mean to hashloop for values here instead of hardcode
+d variable names?
        if ( $month eq "FEB" ) {
            if ( $week eq "W1" ) {
                if (    ( $FEB_L1_W1 <= $Hash{$month}{$week} )
                    and ( $Hash{$month}{$week} <= $FEB_U1_W1 ) )
                {
                    print
"month $month has f status for  $week: $Hash{$month}{$week}\n";
                }
                else {
                    print "error: print something";
                }
            }
            elsif ( $week eq "W2" ) {
                if (    ( $FEB_L1_W2 <= $Hash{$month}{$week} )
                    and ( $Hash{$month}{$week} <= $FEB_U1_W2 ) )
                {
                    print
"month $month has f status for  $week: $Hash{$month}{$week}\n";
                }
                else {
             # print error key/value : $month $Hash{$month} $Hash{$mon
+th}{$week}
             # get exit code
                }
            }
        }
        if ( $month eq "MAR" ) {
            if ( $week eq "W1" ) {
                if (    ( $MAR_L1_W1 <= $Hash{$month}{$week} )
                    and ( $Hash{$month}{$week} <= $MAR_U1_W1 ) )
                {
                    print
"month $month has f status for  $week: $Hash{$month}{$week}\n";
                }
                else {
             # print error key/value : $month $Hash{$month} $Hash{$mon
+th}{$week}
             # get exit code
                }
            }
            elsif ( $week eq "W2" ) {
                if (    ( $MAR_L1_W2 <= $Hash{$month}{$week} )
                    and ( $Hash{$month}{$week} <= $MAR_U1_W2 ) )
                {
                    print
"month $month has f status for  $week: $Hash{$month}{$week}\n";
                }
                else {
             # print error key/value : $month $Hash{$month} $Hash{$mon
+th}{$week}
             # get exit code
                }
            }
        }
    }
}

      [download]
[reply]
[d/l]

        Do I understand correctly that the variables named *_L1_* are lower limits and *_U1_* are upper limits? Although pme has shown code very similar to what I was thinking, the key thing to note is that you can restructure your $FEB_L1_W1 variables into a hash, and then see the similarity in the data structures (Data::Dumper or Data::Dump can also be helpful here).

        my %months = (
    FEB => { W1 => 11.14,
             W2 => 11.22, },
    MAR => { W1 => 33.17,
             W2 => 44.44, },
    APR => { W1 => 55.00,
             W2 => 66.66, },
);
my %limits = (
    FEB => { W1 => { L1 =>  11,    U1 =>  12.05 },
             W2 => { L1 =>  13.06, U1 =>  13.06 }, },
    APR => { W1 => { L1 =>   1,    U1 =>  20    },
             W2 => { L1 =>  33,    U1 =>  13.02 }, },
    MAR => { W1 => { L1 => -12.05, U1 => -14.0  },
             W2 => { L1 =>  22.0,  U1 => -13.02 }, },
);

        [download]

        And then notice you can create loops to loop over both at the same time, like in the following. Note I've made a few assumptions, like that limits for a certain month may not exist, but when they do, both the keys U1 and L1 are defined. If your input data varies, you may have to adjust this.

        for my $mon ( sort keys %months ) {
    for my $week ( sort keys %{ $months{$mon} } ) {
        if ( exists $limits{$mon} && exists $limits{$mon}{$week} ) {
            my $value = $months{$mon}{$week};
            my $upper = $limits{$mon}{$week}{U1};
            my $lower = $limits{$mon}{$week}{L1};
            printf "%6.2f <= %6.2f <= %6.2f - ",
                    $lower,  $value,  $upper;
            if ( $value >= $lower && $value <= $upper ) {
                print "OK\n";
            }
            else {
                print "NOT ok\n";
            }
        }
        else { warn "No limits for month $mon / week $week\n" }
    }
}

        [download]

        Which gives the following output. Note I tweaked the input slightly to give at least one "OK" output.

          1.00 <=  55.00 <=  20.00 - NOT ok
 33.00 <=  66.66 <=  13.02 - NOT ok
 11.00 <=  11.14 <=  12.05 - OK
 13.06 <=  11.22 <=  13.06 - NOT ok
-12.05 <=  33.17 <= -14.00 - NOT ok
 22.00 <=  44.44 <= -13.02 - NOT ok

        [download]
[reply]
[d/l]
[select]
Re: Simplify HoH code
by pme (Monsignor) on Dec 03, 2017 at 11:04 UTC
    Hi greetsathya,

    I reworked your snippet code. It is probably not exactly what you need but I hope you can find useful details in it. 

    #!/usr/bin/perl
use warnings;
use strict;

my @month = qw/JAN FEB MAR APR MAY JUN JUL AUG SEP OCT NOV DEC/;

my $hash = {
  2 => { W1 => 10.14,
         W2 => 11.22,
  },
  3 => { W1 => 33.17,
         W2 => 44.44,
  },
  4 => { W1 => 55.00,
         W2 => 66.66,
  },
};

my $lim = {
  2 => { W1 => { L => 11,    U => 14 },
         W2 => { L => 13,            },
  },
  4 => { W2 => { L => 13.02, U => 15 },
  },
};


for my $month ( sort keys %$hash ) {
  print "$month[$month-1]:\n";
  for my $week ( sort keys %{$hash->{$month}} ) {

    my $lower = defined $lim->{$month}->{$week}->{L} ? $lim->{$month}-
+>{$week}->{L} : 0;   # min value
    my $upper = defined $lim->{$month}->{$week}->{U} ? $lim->{$month}-
+>{$week}->{U} : 999; # max value
    my $value = $hash->{$month}->{$week};

    my $below = ($value < $lower) ?  sprintf('%6.2f', $value) : '';
    my $above = ($value > $upper) ?  sprintf('%6.2f', $value) : '';
    my $in    = ($value >= $lower and
                 $value <= $upper) ? sprintf('%6.2f', $value) : '';

    printf "    %s %6.6s <%6.2f> %6.6s <%6.2f> %6.6s\n",
        $week, $below, $lower, $in, $upper, $above;
  }
}

    [download]
[reply]
[d/l]
      Thanks for your reply. Looking in to your code.
[reply]
Re: Simplify HoH code
by Anonymous Monk on Dec 03, 2017 at 16:04 UTC
    Your "mere page and a half of source code" appears to becoming longer and more-complex the more you try to "improve it." Don't try. What you have written is reasonably easy to understand as-is and the computer won't care. Write a test to thoroughly verify that the existing code works, then close the work-order ticket and move on to the next one.
[reply]
      that doesnt make sense, did you read the question?
[reply]
      A reply falls below the community's threshold of quality. You may see it by logging in.

Back to Seekers of Perl Wisdom