http://www.perlmonks.org?node_id=1200661

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

Hi: I am fairly new to PERL so I apologize for the simplicity of my issue. I'm trying to create an If/Else statement that will look at the contents of two variables from a data extraction ($hashref.) I need to print a blank in a file if the price field is less than 55. I also need the output to be blank if the price field is greater than 55 but the title field contains the word "electric." If the item is greater than 55 without the word electric in the title, I need the phrase "free_batteries" to be output. I've almost got it working. If the item is less than 55, it works, but if the item is greater than 55 with the word "electric" in the field, it's adding "free_batteries" to the record. I'm lost as to what I'm doing wrong. I'm sure this is a simple fix for someone who knows PERL. Any help you can lend is greatly appreciated. - Tom 

$tempdesc = $hashref->{price};
$tempdesc2 = $hashref->{title};

    if ($tempdesc >= 55)
    {      
       if ($tempdesc2 =~ /electric/) {print FH_OUTFILE "\n";}
       else {print FH_OUTFILE "free_batteries\n";}}
    else {print FH_OUTFILE "\n";}
    }

[download]

Replies are listed 'Best First'.
Re: If statement issue from a Noob
by haukex (Archbishop) on Oct 04, 2017 at 15:50 UTC

    Try running this script, I hope it is self-explanatory, if not, please ask!

    #!/usr/bin/env perl
use warnings;
use strict;
use Test::More;

# I need to print a blank in a file if the price field is
# less than 55.
is output(54,'title'), "\n";
is output(54,'electric'), "\n";

# I also need the output to be blank if the price field is
# greater than 55 but the title field contains the word "electric."
is output(56,'the electric title'), "\n";

# If the item is greater than 55 without the word electric
# in the title, I need the phrase "free_batteries" to be output.
is output(56,'title'), "free_batteries\n";

# what about these two?
is output(55,'title'), "free_batteries\n";
is output(55,'electric'), "\n";

sub output {
    my $price = shift;
    my $title = shift;
    if ( $price < 55 || $title =~ /electric/ ) {
        return "\n"
    }
    else {
        return "free_batteries\n";
    }
}

done_testing;

    [download]
[reply]
[d/l]
Re: If statement issue from a Noob
by pryrt (Abbot) on Oct 04, 2017 at 16:18 UTC

    haukex++'s solution is good, and asks an important question, since your specs were not internally consistent regarding exactly 55. However, in case you were curious about your logic specifically:

    Your version doesn't compile for me as-is: you have an unmatched curly bracket. Clean up the formatting to only do one thing per line, and you should see it:

    if ($tempdesc >= 55)
{
    if ($tempdesc2 =~ /electric/)
    {
        print FH_OUTFILE "\n";
    }
    else
    {
        print FH_OUTFILE "free_batteries\n";
    }
}
else
{
    print FH_OUTFILE "\n";
}
}

    [download]

    After removing that final } (shown in the SSCCE below): it seems to me to do what you want (assuming that "greater than 55" means "greater than or equal to 55", to match your >=).

    use warnings;                                                         
+                      # ALWAYS!!!
use strict;                                                           
+                      # ALWAYS!!!

open FH_OUTFILE, '>&', \*STDOUT or die "err dup STDOUT = $!";         
+                      # added for SSCCE

foreach my $hashref(                                                  
+                      # added for SSCCE
    {price => 55, title => 'i am expensive electric, and want a blank'
+ },                   # added for SSCCE
    {price => 55, title => 'i am expensive gasoline, and want to print
+ something else'},    # added for SSCCE
    {price => 54, title => 'i am cheap electric, and want a blank' }, 
+                      # added for SSCCE
    {price => 54, title => 'i am cheap gasoline, and want a blank' }, 
+                      # added for SSCCE
) {                                                                   
+                      # added for SSCCE
    my $tempdesc = $hashref->{price};
    my $tempdesc2 = $hashref->{title};

    print FH_OUTFILE "-"x40, " START\n", "TITLE = '$tempdesc2' \@ PRIC
+E = '$tempdesc'\n";   # added for SSCCE
    if ($tempdesc >= 55)
    {
        if ($tempdesc2 =~ /electric/)
        {
            print FH_OUTFILE "\n";
        }
        else
        {
            print FH_OUTFILE "free_batteries\n";
        }
    }
    else
    {
        print FH_OUTFILE "\n";
    }
    print FH_OUTFILE '.'x40, " END\n";                                
+                      # added for SSCCE
}                                                                     
+                      # added for SSCCE

__END__
---------------------------------------- START
TITLE = 'i am expensive electric, and want a blank' @ PRICE = '55'

........................................ END
---------------------------------------- START
TITLE = 'i am expensive gasoline, and want to print something else' @ 
+PRICE = '55'
free_batteries
........................................ END
---------------------------------------- START
TITLE = 'i am cheap electric, and want a blank' @ PRICE = '54'

........................................ END
---------------------------------------- START
TITLE = 'i am cheap gasoline, and want a blank' @ PRICE = '54'

........................................ END

    [download]

    If our outputs are not your desired output, you'll need to clarify.

[reply]
[d/l]
[select]
Re: If statement issue from a Noob
by 1nickt (Canon) on Oct 04, 2017 at 16:50 UTC

    Hi, in plain human language you are saying "free batteries only with orders worth $55 or more for non-electric items".

    So say that in Perl: 

    use strict; use warnings;
...
open my $OUT, '>', $filename or die $!;
print $OUT "free_batteries" if $price >= 55 and $title !~ /electric/;
print $OUT "\n"; # in all cases

    [download]

    Update: Changed to use postfix conditional to avoid any debate over brackets, braces or indentation ;-)


    The way forward always starts with a minimal test.
[reply]
[d/l]
      And who needs batteries for a non-electric item? I think the OP wants something different. Free batteries for > $55 electric items.


      holli

      You can lead your users to water, but alas, you cannot drown them.
[reply]
[d/l]

        I'm pretty sure the OP said:

        I also need the output to be blank if the price field is greater than 55 but the title field contains the word "electric."

        If the item is greater than 55 without the word electric in the title, I need the phrase "free_batteries" to be output.


        The way forward always starts with a minimal test.
[reply]
Re: If statement issue from a Noob
by Laurent_R (Canon) on Oct 04, 2017 at 17:50 UTC
    Hi tomdee27,

    if you were formatting your code correctly (i.e. with consistent indentation), you would probably see your mistake.

    Both indentation styles shown below, used consistently, show clearly that you have a spurious extra closing curly at the end: 

    if ($tempdesc >= 55) {      
    if ($tempdesc2 =~ /electric/) {
        print FH_OUTFILE "\n";
    } else {
        print FH_OUTFILE "free_batteries\n";
    }
} else {
    print FH_OUTFILE "\n";
}
}

    [download]
    And: 
    if ($tempdesc >= 55)
{      
    if ($tempdesc2 =~ /electric/) 
    {
        print FH_OUTFILE "\n";
    } else 
    {
        print FH_OUTFILE "free_batteries\n";
    }
} else 
{
    print FH_OUTFILE "\n";
}
}

    [download]
    Even though the compiler does not care about correct (consistent) indentation, human readers do. And that includes myself, other monks around here and, last but not least, YOU. The careless formatting of the code in your original post prevents you from seeing your error.

    Taking the time to carefully and consistently indent your code (using whichever indenting style you may want to choose) is never a waste of time, and it is most of the time a great time saver. Indentation may not be needed by the compiler, but it is not optional, it is essential for the human reader (you in the first place).

    You're obviously a beginner and may not have figured out this point until now, but, please, please, take right now the good habit for formatting correctly your code.

[reply]
[d/l]
[select]
Re: If statement issue from a Noob
by holli (Abbot) on Oct 04, 2017 at 19:05 UTC
    Do yourself a favour (additionally to care for proper indentation) and use descriptive variable names; 
    $price = $hashref->{price};
$title = $hashref->{title};

    [download]
    instead of 
    $tempdesc = $hashref->{price};
$tempdesc2 = $hashref->{title};

    [download]
    Saves you a lot of looking up, wondering and bug hunting.


    holli

    You can lead your users to water, but alas, you cannot drown them.
[reply]
[d/l]
[select]

      Thanks to everyone for your suggestions. I was able to use a combination of suggestions to get the solution. I apologize in advance for the bad formatting, but here's what I used:

      $tempdesc = $hashref->{price};
$tempdesc2 = $hashref->{title};

if ($tempdesc >= 55 && $tempdesc2 !~ /electric/) 
{
print FH_OUTFILE "free_batteries\t";
} 
{
print FH_OUTFILE "\n";
}

      [download]
[reply]
[d/l]

        Note that the extra set of braces around print FH_OUTFILE "\n"; makes it look at first glance like you're missing an else when you're actually not (since it seems this is based on 1nickt's initial code), I recommend removing them. Also, you should take holli's advice (and as several others have shown) to use more descriptive variable names.

        I apologize in advance for the bad formatting

        Why apologize when you can fix it? ;-) Or at least have perltidy do it for you.

[reply]
[d/l]
[select]
        Still not rightly formatted, the print FH_OUTFILE "free_batteries" should be indented compared to the if statement: 
        $tempdesc = $hashref->{price};
$tempdesc2 = $hashref->{title};

if ($tempdesc >= 55 && $tempdesc2 !~ /electric/) 
{
    print FH_OUTFILE "free_batteries";
} 
print FH_OUTFILE "\n";

        [download]
        The reason may not be very obvious here, but if you have several statements in the body of the if block, then you can make much more visually obvious what depends on the condition in the if statement, and what not: 
        if ($tempdesc >= 55 && $tempdesc2 !~ /electric/) 
{
    print FH_OUTFILE "free_batteries\n";
    print FH_OUTFILE "something else\n";
    print "A message displayed on the screen if the condition is satis
+fied\n";
} 
print FH_OUTFILE "\n";

        [download]
        With this, anyone can see clearly that the three print statements between the curlies should be executed only if the condition is met, while the last print statement after the closing curly will run anyway.

        Besides that, I agree with what other monks have said: use meaningful variable names, use strict; and use warnings;, and declare your variables with the my keyword. Also use lexical file handles. So your full code could end up being something like that: 

        use strict;
use warnings;

# here the code where the $hashref is populated (or maybe it is actual
+ly part of a hash of hashes or array of hashes, we don't know enough 
+details)

open my $FH_OUTFILE, ">", "output.txt" of die "could not open the outp
+ut file $!";

my $price = $hashref->{price};
my $item_name = $hashref->{title};

if ($price >= 55 and $item_name !~ /electric/) 
{
    print $FH_OUTFILE "free_batteries";
}
print $FH_OUTFILE "\n";
# perhaps some code to close $FH_OUTFILE (if you're done at this point
+)

        [download]
[reply]
[d/l]
[select]

Back to Seekers of Perl Wisdom