Re: Little annoying mistakes ... of others
by ikegami (Patriarch) 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
| [reply] [d/l] [select] |
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. | [reply] [d/l] [select] |
|
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.
| [reply] |
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
| [reply] [d/l] [select] |
|
for (my $i = 0; $i < $#names; $i++)
{
print $names[$i];
}
| [reply] [d/l] |
|
for (my $i = 0; $ <= @names; $i++)
{ print $names[$i] }
Given the C code. Hey, sometimes Perl is useful as a C prototyper.
| [reply] [d/l] |
|
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:
- Does it work?
- Can someone else come in, make a change, and be reasonably certain no bugs were introduced?
| [reply] [d/l] [select] |
|
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.
| [reply] [d/l] [select] |
|
I think the point is efficiency (avoid copying the argument) rather than counting characters.
| [reply] |
|
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. :)
| [reply] |
|
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.
| [reply] [d/l] [select] |
|
|
|
|
And where is the mistake?
| [reply] |
|
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
| [reply] |
Re: Little annoying mistakes ... of others
by toolic (Bishop) 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. | [reply] [d/l] [select] |
|
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!
| [reply] [d/l] [select] |
|
| [reply] |
|
|
| [reply] |
|
if ($foo eq 'goo' || 'moo') { ... }
Error that may be but I do wish somethimes that that ought to be correct.
| [reply] [d/l] |
|
| [reply] |
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.
| [reply] [d/l] |
|
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
| [reply] [d/l] |
Re: Little annoying mistakes ... of others
by Your Mother (Archbishop) 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."
| [reply] [d/l] |
Re: Little annoying mistakes ... of others
by Bloodnok (Vicar) on Dec 06, 2008 at 17:59 UTC
|
| [reply] |
Re: Little annoying mistakes ... of others
by ikegami (Patriarch) 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
| [reply] [d/l] [select] |
my @ARGV
by szabgab (Priest) on Dec 08, 2008 at 20:30 UTC
|
| [reply] |
|
So does my $_; and my $var;. Is there a case where my doesn't create a new variable?
| [reply] [d/l] [select] |
|
| [reply] [d/l] [select] |
|
|
|
|
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. | [reply] |
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
| [reply] |
|
| [reply] [d/l] |
|
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
| [reply] |
|
|
|
| [reply] |
|
| [reply] |