Re: "Useless use of a constant" in grep block?
by Athanasius (Archbishop) on Jul 30, 2017 at 06:02 UTC
|
Hello perlancar,
Tangential observation:
push @ary, "foo" unless grep { $_ eq "foo" } @ary;
I think either the any or the none function in the core List::Util module would be better than grep in this case:
push @ary, "foo" unless any { $_ eq "foo" } @ary;
# OR
push @ary, "foo" if none { $_ eq "foo" } @ary;
Not only are these clearer (since the reader doesn’t have to work out “what is grep returning here?”), but they’re also more efficient because — unlike grep — they short-circuit as soon as a match is found.
Hope that’s of interest,
| [reply] [d/l] [select] |
|
|
I'm aware of List::Util's any/none/first. Depending on the size of the list, grep or do { for ... } can be faster than List::Util's offerings because there is no subroutine call and argument passing overhead. Also there's a shortcutting variant of grep using eval { grep { ... and die } ... } idiom. I made a benchmark on this in Bencher::Scenario::PERLANCAR::grep_bool.
| [reply] |
Re: "Useless use of a constant" in grep block?
by shmem (Chancellor) on Jul 30, 2017 at 10:10 UTC
|
Well, this
push @ary, "foo" unless grep { "foo" } @ary;
is a funny way to say
push @ary, "foo" if 0 == @ary;
so it is just TIMTOWTDI ;-)
perl -le'print map{pack c,($-++?1:13)+ord}split//,ESEL'
| [reply] [d/l] [select] |
Re: "Useless use of a constant" in grep block?
by LanX (Saint) on Jul 30, 2017 at 10:16 UTC
|
It's not useless, "foo" is true in Perl !
It's reasonable to allow a grep to be changed in a way that it greps the whole list.
Since there is no explicit TRUE or FALSE statement in Perl, one can't tell if a literal string was just meant to be true or could be a typo.
Like already said, adding too many rare edge cases to Perl's error detection would make the code base unnecessarily complicated.
| [reply] |
Re: "Useless use of a constant" in grep block?
by Anonymous Monk on Jul 30, 2017 at 02:48 UTC
|
Would it be reasonable for me to expect perl, in the first case, to emit a warning. Say something like "Probably useless use of a constant in grep block"?
Sure its reasonable, but then lots of things are reasonable :)
How often would you make this error? How often would you not notice that @ary didn't get filtered? Average programmer?
I think rarely * rarely * rarely = warning unimportant Also, "Probably" doesn't really belong in warnings :) it means this shouldn't be a warning
| [reply] |
|
|
I tend to agree on the relatively low frequency that people might make this kind of mistake. But:
Also, "Probably" doesn't really belong in warnings :) it means this shouldn't be a warning
I beg to differ. Perl has quite a few warnings of this very nature, where a syntax is perfectly valid but probably not what the programmer meant. Some examples (see perldiag for more details and a more complete list):
Possible attempt to put comments in qw() list
Possible attempt to separate words with commas
Possible memory corruption: %s overflowed 3rd argument
Possible precedence issue with control flow operator
Possible precedence problem on bitwise %s operator
Possible unintended interpolation of $\ in regex
/%s/ should probably be written as "%s"
| [reply] |
|
|
I beg to differ. Perl has quite a few warnings of this very nature, where a syntax is perfectly valid but probably not what the programmer meant. Some examples (see perldiag for more details and a more complete list):
So sorry but you're wrong :)
Just cause perl has some that use "probably" doesn't mean they belong in there, cause it doesn't :D they're humans just like me
Specifically this doesn't belong but its super super old
Possible attempt to put comments in qw() list
Possible attempt to separate words with commas
I'm sure the larry or whomever could come up with the reason for including it, but I think its stupid cause how can you be surprised when you type that stuff up and then @foo ends up contains both "#" and "," ? First time you run the program you discover , hey, qw works exactly as advertised, who knew the manual doesn't lie but no , instead you get annoying warning, cause some tiny percent of learners (0.0000001%) don't know basic debugging (verify the contents of your variables).
This has annoyed me since I first learned perl. warnings.pm is for work not 5th grade programming class .
This one should be phrased better than Possible memory corruption: %s overflowed 3rd argument but whats done is done
I think this is better memory overflow: %s overflowed 3rd argument cause if "memory overflow" doesn't tell you "uh-oh" then surely use diagnostics; would explain it may be "possible memory corruption"
Possible precedence issue with control flow operator should just be a fatal syntax error , cause there ain't no reason other than obfuscation to allow this nonsense
Another wording problem Possible unintended interpolation of %s in string, Should read Cannot interpolate %s in string
This should be fatal /%s/ should probably be written as "%s" cause obfuscation/japh is the only reason to this nonsense join m/word/ , 1,2,3,4
Possible unintended interpolation of $\ in regex should be fatal cause it doesn't warn on ther "metacharacter" global variables
And my favorite Possible precedence problem on bitwise %s operator should read DAMN SON YOU MUST BE A PRECEDENCE GENIUS TO WRITE %s with bitwise %s operator. THE LARRY SALUTES YOU!!
warnings is warnings its not extra linty warnings
But yeah what do i know | [reply] [d/l] [select] |
|
|
|
|
|
|
Re: "Useless use of a constant" in grep block?
by haukex (Archbishop) on Jul 31, 2017 at 15:28 UTC
|
From my casual lurking on P5P, I think getting a warning like that added to the core might not be easy, or it'll take a while, and then only be available in the newer versions of Perl. But what you can do instead is use this Perl::Critic policy that I whipped up:
When run against your code with perlcritic -3 code.pl, it'll say something like ""grep" with constant value at line X, column Y. Will return all or none of the values."
You can monkey-patch this into your installation by running perldoc -l Perl::Critic::Policy::BuiltinFunctions::ProhibitComplexMappings, taking that pathname, and placing the above code into that path as the file GrepWithSimpleValue.pm. (Maybe someday I'll get around to making a distro, along with my other policy :-))
| [reply] [d/l] [select] |
|
|
| [reply] |
Re: "Useless use of a constant" in grep block?
by ForgotPasswordAgain (Vicar) on Jul 30, 2017 at 16:15 UTC
|
I think a warning would at least not be unreasonable there. Maybe bring it up on perl5-porters@ . | [reply] |
Re: "Useless use of a constant" in grep block?
by kcott (Archbishop) on Jul 30, 2017 at 05:37 UTC
|
G'day perlancar,
I was unable to reproduce your warning ("Useless use of a constant") or, indeed, any warnings:
$ perl -wE 'my @ary = qw{a b}; push @ary, "foo" unless grep { "foo" }
+@ary; say "@ary"'
a b
I'm using 5.26.0.
Thinking it might be version-related, I also tried the same line with
5.24.0,
5.22.0,
5.20.0,
5.18.0, and
5.14.0.
None produced any warnings; all produced identical output: "a b".
So, either you're using some other version (please state which),
or the code you posted is not the code that generated that warning.
The constant "foo" should be perfectly valid as a condition in the grep block
(evaluating to TRUE).
Similarly, a FALSE constant would be just as valid.
$ perl -wE 'my @ary = qw{a b}; push @ary, "foo" unless grep { "0" } @a
+ry; say "@ary"'
a b foo
Perl often provides helpful hints in its warning messages:
"Missing semicolon ...", "Might be a runaway multi-line ...", and so on.
If your message is from an old version of Perl, there's little you can do about that (beyond upgrading, of course).
If your message is from different code to that shown,
suggestions for improvements can be made when we know what that code is.
| [reply] [d/l] [select] |
|
|
So, either you're using some other version (please state which), or the code you posted is not the code that generated that warning.
Or option 3: The point of the OP wasn't "why did I get this warning?", but, rather, "I made this mistake and didn't get a warning. Maybe it should issue one?"
| [reply] |
|
|
Firstly, my apologies for the late reply.
I usually log in daily; this week I skipped a couple of days.
I read the quote in the title ("Useless use of a constant") as the message received after making
"a stupid mistake"; and the quote in the body ("Probably useless use of a constant ...")
as the preferred message.
On rereading, I think you're right and my interpretation was incorrect.
| [reply] |
Re: "Useless use of a constant" in grep block?
by ikegami (Patriarch) on Jul 31, 2017 at 18:45 UTC
|
"Probably useless use of a constant in grep block" is a step in the wrong direction. There is definitely an error, and it's due to the use of a constant.
One could say that it's the grep that useless, but that's moving away from the cause of the problem, and that's only the case for true constants. In fact, it's impossible to talking in terms "useless use" for false constants in a grep block.
However, using "useless use" in the error message provides familiarity. Combined with identifying the constant as the source of the problem, this message allows us to identify the problem more easily than a more accurate message would.
| [reply] |