Beefy Boxes and Bandwidth Generously Provided by pair Networks BBQ
Keep It Simple, Stupid
 
PerlMonks  

Little annoying mistakes ... of others

by szabgab (Priest)
on Dec 06, 2008 at 17:11 UTC ( #728569=perlmeditation: print w/ replies, xml ) Need Help??

When I started to write this entry I noticed the your Perl bug Achilles heel by perrin which is similar but not really.
I am looking for bugs that you might not make any more but either you used to make or you know others often make it. Little annoyances that can easily trip the unaware without perl warning them.

While reading the Minimal Perl by Tim Maher I encountered two warnings about mistakes beginners might make. Based on those two I started to collect some little mistakes people can make in Perl but that perl does not catch.

The first two are based on the warnings from Minimal Perl:

chomp does not return the string

print "type in something: "; my $line = <STDIN>; $line = chomp $line;
The correct way of course would be to just write:
print "type in something: "; my $line = <STDIN>; chomp $line;

substitute returns true/false and not the changed string even in map

my @data = qw(foo bar baz); @data = map { s/./X/ } @data; print "@data\n";
The above prints 1 1 1

The correct way would be to let map return the changed value instead of the success/failure code of the substitution.

my @data = qw(foo bar baz); @data = map { s/./X/; $_} @data; print "@data\n";

|| has higher precedence than ,

my $filename = 'input.txt'; open my $fh, '<', $filename || die $!;
|| being higher precedence that above is the same as
open my $fh, '<', ($filename || die $!);
which means first the || is evaluated which will always be true (except of the stupid edge cases such as $filename being empty string or 0) so in normal circumstances everything will work but if the open fails, it still won't be reported.

The correct would be to either use or instead of || or put parentheses around the parameters of open(). Personally I try to write and teach this form:

my $filename = 'input.txt'; open(my $fh, '<', $filename) or die $!;

Perl 6-like constructs work in perl 5

A student of mine had to iterate over the values of an array so he wrote:
my @names = qw(A B C); foreach my $name (<@names>) { print "$name\n"; }
The annoying part here is that this works despite being incorrect.

We are talking about Perl 5 here!.

Using perl -MO=Deparse example.pl reveals what perl thought about the above code:

my(@names) = ('A', 'B', 'C'); use File::Glob (); foreach my $name (glob(join($", @names))) { print "$name\n"; }

magic $_ regex matching

my $x = 42; if ($x = /x/) { print "ok\n"; }
That will try to match $_ with /x/
If you are lucky you get Use of uninitialized value $_ in pattern match (m//) at code.pl line 32.. If you are unlucky and $_ already had a value the above will make the mistake silently.

I am not saying it is not legitimate code. I am saying I see beginners write that by mistake without understanding what that means.

or is weaker than return

Based on the example given by rhesa
my $x = 42; print is_foo_or(); sub is_foo_or { return foo($x) or $x; } sub foo { return 0; }
The above will return 0 despite it being false and will do it silently.

Question

So what are your annoyances or rather those bugs that you see others make while you think they are simple?

Comment on Little annoying mistakes ... of others
Select or Download Code
Re: Little annoying mistakes ... of others
by ikegami (Pope) on Dec 06, 2008 at 17:52 UTC

    A problem with your question, not an answer to your question:
    @data = map { s/./X/; $_} @data;
    is incorrect too. Redundant, at least. It does the same as
    map { s/./X/ } @data;
    which is a poor way (in my opinion) to write
    s/./X/ for @data;

    If you're setting up a chain, you'd want one of
    map { my $s = $_; $s =~ s/./X/; $s }
    apply { s/./X/ }   # From List::MoreUtils
    Filter { s/./X/ }  # From Algorithm::Loops

Re: Little annoying mistakes ... of others
by Bloodnok (Vicar) on Dec 06, 2008 at 17:59 UTC
    Is it me, or is this OP more a candidate for a meditation (c/w SoPW) ... or maybe even Q&A ??

    Just asking ;-)

    A user level that continues to overstate my experience :-))
Re: Little annoying mistakes ... of others
by Lawliet (Curate) on Dec 06, 2008 at 18:02 UTC
    my $line = <STDIN>; chomp $line;

    Can be written chomp(my $line = <STDIN>); ;D

    Anyway, a pet peeve of mine would be people writing a program in Perl as if they were writing a program in C (or C++, etc). I.e., using C-style for loops when a foreach would work much better. Or, something even more annoying, putting the curlies in the same vertical column~

    for (my $i = 0; $i < $#names; $i++) { print $names[$i]; }

    I'm so adjective, I verb nouns!

    chomp; # nom nom nom

        Worse? That's the One True Way. However, for one-liners, horizontal alignment is preferable, i.e.:

        for (my $i = 0; $ <= @names; $i++) { print $names[$i] }

        Given the C code. Hey, sometimes Perl is useful as a C prototyper.

        sas
        The funniest part is that all three for-loops (parent, this, and child) are wrong (which only is a great defense of the whole point). The correct versions are
        for (my $i = 0; $i <= $#names; $i++) { ... } # or for (my $i = 0; $i < @names; $i++) { ... }
        You guys had $i < $#names and $i <= @names. :-)

        My criteria for good software:
        1. Does it work?
        2. Can someone else come in, make a change, and be reasonably certain no bugs were introduced?
      Can be written chomp(my $line = <STDIN>); ;D

      But that doesn't generalise for all rvalue uses. For example, I frequently want to write something like

      frobnicate(chomp) while <$file>;     # doesn't work

      and am of course instead forced to write the loop out longhand, since

      chomp, frobnicate($_) while <$file>; # yuck

      is starting to get hard to read.

      Personally I consider this a failure in Perl's Huffman coding. I can't think of a single case where I've cared in the slightest how many characters chomp removed, but I do want to be able to chomp something and then immediately pass the chomped value to some other routine.

        I think the point is efficiency (avoid copying the argument) rather than counting characters.
      To contrast, what annoys me, is people putting curlies at the end of a line where they are hard to find, and even harder to match up with their close curly.

      I have no idea why saving one line could be worth reducing readability like that, even on an 800x600 screen like my laptop. :)

        Why do you need to find them? There's a while, for, eval, if, sub or something like that at the start of the line and the following ones are indented.

      And where is the mistake?
        So what are your annoyances or rather those bugs that you see others make while you think they are simple?

        I answered that part. I hope you are not too mad :(

        Although, it can be considered a mistake according to the rules. Sort of.

        I'm so adjective, I verb nouns!

        chomp; # nom nom nom

Re: Little annoying mistakes ... of others
by tprocter (Sexton) on Dec 06, 2008 at 19:41 UTC
    As part of my job, I review code by people that are less then experienced with Perl. I've seen some bad code such as:
    open(F, "grep 'WARNING<\/font>' $mydir/$myfile".".|wc|awk '{print \$1} +'| ") ; while (<F>) { chomp(); $received_warn=$_; } close F;

    Which is part of a larger script where the same file was opened and closed several times and processed as shown by scanning the entire file, looking for specific html, then counting rows in the output file for the condition. Meanwhile, all of the information needed for all of the processing was available from a single pass of the file head.

    However, I think the most common mistake people make is confusing numeric comparisons with string comparisons. This kind of mistake often passes simple manual tests but breaks if it gets to production.

    Example:
    $a = 'a'; if ($a == 'b' or $a == 'c') { print "True\n"; }
    ...or even worse:
    if ($a = 'b') { print "True\n"; }
    Both examples print 'True', but the last example has corrupted your data.

      It looks to me like the thing most of the people you are complaining about forget to always use strictures (use strict; use warnings; - see The strictures, according to Seuss).

      They should also learn not to use $a and $b in general use ($foo and $bar are good for example code).

      Learning to use the three parameter form of open, lexical variables for file handles and checking the result of the open would seem to be a higher level of sophistication. However using comprehensible and consistent indentation should be attainable for them, even if just by using Perl tidy.


      Perl's payment curve coincides with its learning curve.
Re: Little annoying mistakes ... of others
by Limbic~Region (Chancellor) on Dec 06, 2008 at 22:34 UTC
    szabgab,
    I am sure I could come up with a laundry list but my mind is on other things (family isn't feeling well). The one that bugs me is not considering defined values that evaluate to false when setting up a conditional. Since I am guilty of this, let me give an example:
    #!/usr/bin/perl use strict; use warnings; my $file = $ARGV[0] or die "Usage: $0 <input_file>"; open(my $fh, '<', $file) or die "Unable to open '$file' for reading: $ +!";

    While I think it is uncommon that someone would name their file '0', it is possible. In other examples, the oversight might be the empty string ''. As a reference, see True and False (aka Booleans). I know that 5.10 introduced the // operator which is great.

    Cheers - L~R

      Along the sames lines, and much more likely, is to pass in a numeric option on the command line whose value is quite probably 0. For example, I have a histogram script in which I can pass in the minimum value. Unless I check the value using defined, the thing just doesn't work right:
      use strict; use warnings; use Getopt::Std; our $opt_m; getopts('m:'); print "$opt_m\n" if ($opt_m); # Wrong #print "$opt_m\n" if (defined $opt_m); # Right
Re: Little annoying mistakes ... of others
by Your Mother (Canon) on Dec 07, 2008 at 02:16 UTC

    To extend what ikegami said above-

    substitute returns true/false and not the changed string even in map

    Substitute does not return true/false. It returns the count of substitutions performed.

    perl -le '$str = "A.B.C."; $x = $str =~ s/\w//g; print $x' 3 # or a "1" without the "g."
Re: Little annoying mistakes ... of others
by toolic (Chancellor) on Dec 07, 2008 at 03:08 UTC
    A pretty common logic error is forgetting to explicitly check all expected values against the variable:
    use strict; use warnings; my $foo = 'boo'; if ($foo eq 'goo' || 'moo') { print "$foo\n"; }

    when this is really desired:

    if ($foo eq 'goo' || $foo eq 'moo') {

    Obviously, this is not unique to Perl, but I do see it quite often.

      This is exactly the type of examples I am looking for.

      Funny as I have just seen this happen in my last class 4 days ago.

      if ($foo eq 'goo' || 'moo') { ... }
      Error that may be but I do wish somethimes that that ought to be correct.
        goto Perl 6;

      Actually ... i would desire this:

      use strict; use warnings; my $arg = shift || 'boo'; my @valid = qw( goo moo foo voo doo poo ); print "$arg matches\n" if grep /$arg/, @valid;

      But this bring up an annoyance as well, because this mistake will compile and always return true (as long as the array being checked is not empty):

      # wrong way ahead! print "$arg matches\n" if grep $arg, @valid;
      Oops!

      jeffa

      L-LL-L--L-LL-L--L-LL-L--
      -R--R-RR-R--R-RR-R--R-RR
      B--B--B--B--B--B--B--B--
      H---H---H---H---H---H---
      (the triplet paradiddle with high-hat)
      
        Wow, I am writing down the examples as test cases but I can't figure out why is the second example always true?

        could you please explain?

Re: Little annoying mistakes ... of others
by matze77 (Friar) on Dec 07, 2008 at 09:58 UTC

    Since i am a beginner here are some of the mistakes i made: (to be continued for sure):
    forget "#" for a comment, unfortunately at the same line as i try to use a new function searching the syntax error
    missing ";" at the end of the line. missing closing "}"
    Defining variables like $count and try to use variable $cont some lines down (and searching the typo for minutes ;-)).

    Maybe i should use Padre. http://padre.perlide.org/ MH

      Defining variables like $count and try to use variable $cont some lines down (and searching the typo for minutes ;-)).

      use strict; will finds those immediately.

        Yes normally i do, but i took a example (afair regarding while loop from Lama Book) which was modified, and it didnt run first cause there seemed to be a problem with a variable and the "use strict;" so i turned it off ;-), i could list the example as soon as i find it in the book again

        Update: My fault. I noticed that use strict; and the declaration e.g. with "my" is introduced later in another chapter so i was not supposed to use strict; at all in this example (in chapter 3) of the book, i didnt know how to "declare" the variable so i uncommented use strict; for a quick and dirty solution. /Update: MH
      I appended your comments to the ticket of Padre.

      I'd be glad to get clarifications, or more such annoyances!

Re: Little annoying mistakes ... of others
by ikegami (Pope) on Dec 08, 2008 at 01:58 UTC

    Omitting parens around argument lists has issues. You've already mentioned the problem with omitting parens in conjunction with || instead of or. There's also

    print (3+4)*5; # Prints 7

    At least that one warns.

    >perl -we"print (3+4)*5" print (...) interpreted as function at -e line 1. Useless use of multiplication (*) in void context at -e line 1. 7
my @ARGV
by szabgab (Priest) on Dec 08, 2008 at 20:30 UTC
    Peter Scott suggested on Perl 5 Porters to warn if a user tries to declare my @ARGV as currently it just silently hides the real @ARGV.

    There are some more issues in that thread on the mailing list.

      So does my $_; and my $var;. Is there a case where my doesn't create a new variable?
        my $_ is new with 5.10.0, and switches anything in the scope that by default is using the global $_ to using the lexical $_ instead.

        my @ARGV has no such magic. readline or eof will continue to default to using the global @ARGV, even though there's a lexical @ARGV in scope. This is the reason for a warning.

Re: Little annoying mistakes ... of others
by tinita (Parson) on Dec 09, 2008 at 18:50 UTC
    I have collected some "Don't"s in my lightning talk from Copenhagen: http://tinita.de/docs/slides/don_t/slides.vroom.
    These are mistakes I made myself or found while refactoring code from others, or when answering perl questions in forums or IRC.
    edit: It's a bit short, so my example why one shouldn't use C-style for loops is missing the reason why I decided to include it - I found an off-by-one error. And of course every rule has its exceptions.

Log In?
Username:
Password:

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

How do I use this? | Other CB clients
Other Users?
Others examining the Monastery: (5)
As of 2014-04-20 09:00 GMT
Sections?
Information?
Find Nodes?
Leftovers?
    Voting Booth?

    April first is:







    Results (485 votes), past polls