The maybe it is better written this way tool

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

Replies are listed 'Best First'.
Re: The maybe it is better written this way tool by toolic (Bishop) on Nov 15, 2009 at 00:34 UTC
a few examples that I don't think Perl::Critic or any of its extensions covers `my $x; foreach $x () {}` [download] I uploaded your 2 lines of code to the free Perl::Critic website, http://perlcritic.com/, and it produces this message: `Loop iterator is not lexical at line 2, column 1. See page 108 of PBP +. Severity: 5` [download] It even has a link to the verbose explanation of the message, complete with advice on how to re-code it. Therefore, Perl::Critic does indeed detect this coding style. I know Perl::Critic is configurable, and I would not be surprised to find out that it is also extensible.	[reply] [d/l] [select]
Re^2: The maybe it is better written this way tool by eclpmb (Hermit) on Nov 15, 2009 at 11:39 UTC
Perl::Critic is extendable. To do so you write your own Perl::Critic::Policy module. Perl::Critic::DEVELOPER has documentation on doing so.	[reply]
Re: The maybe it is better written this way tool by ikegami (Patriarch) on Nov 14, 2009 at 22:03 UTC
@files = `ls .xml\|more`;* This example is pretty far fetched. You're asking for a tool to assist Perl programmers that understands (correctly or otherwise) that the shell command `ls .xml \| more` [download] is a bad way of writing `echo .xml` [download] A tool that converts chomp( my @files = `echo .xml` ); [download] to the better `my @files = glob('.xml');` [download] is already pretty far fetched. Yet another example would be the extensive processing of values from @ARGV that could be replaced by the use of Getopt::Long. Seems to me that "processing @ARGV" is hard to quantify into a test. That said, there does exist a framework for notifying users of bad code. It's Perl::Critic. It doesn't modify the code since the idea is to identify as many possible problems as possible. You'd expect false positives here. Presumably a user willing to use Perl::Critic will look at the documentation to figure out what the warning he gets means. The recommended change can be placed there. Now I notice you mentioned Perl::Critic. You seem to dismiss it. Why is that? Because no one coded all your suggestions yet? ( Apparently, it already checks for your first example )	[reply] [d/l] [select]
Re^2: The maybe it is better written this way tool by szabgab (Priest) on Nov 15, 2009 at 07:13 UTC
Err, I don't understand what you meant by far fetched? That it will be difficult to pro-grammatically understand the intention? Yes I guess it will. Did you mean something else? "processing @ARGV" is hard to quantify into a test. That's why I am asking here for suggestions but in what to check and if it seems to be possible. Regarding @ARGV I would look for 2 or more occurrences of either \$ARGV\[ or shift; outside of any sub or shift @ARGV; and then recommend. Funny how easily we can misunderstand each other. I mentioned Perl::Critic in my post in order to acknowledge it and to show I am aware of it and not in order dismiss it. Not at all. Though I have not checked Perl::Critic for these I am looking for things that are probably not covered by Perl::Critic (yet?) as they are not really Perl coding styles. If the implementation will be another extension of Perl::Critic or something else is another matter. It would be probably easier for the user (and for the Padre integration) if it was built on top of Perl::Critic. As it was pointed out by others at least one of the examples I gave is already covered by Perl::Critic. I should read the book again...	[reply]
Re^3: The maybe it is better written this way tool by ikegami (Patriarch) on Nov 23, 2009 at 19:42 UTC
I don't understand what you meant by far fetched? Unless you were suggesting the tool should only look for that specific string, the tool would have to know That `more`'s output is being piped. How `more` behaves when it's output is being piped. The directory from which `ls` is launched. That the file spec will only match the names of file (not directories). That `ls` is given a list of file names (not directories). How `ls` behaves when given a list of file names. It would also have to make assumptions such as `sh` is used as the shell. `ls` and `more` aren't aliases. `ls` and `more` refer to the standard utilities. All this from a program that's suppose to know Perl. And that's just to handle that one command. That it will be difficult to pro-grammatically understand the intention? Yes I guess it will That it requires an incredible amount of knowledge about non-Perl material to have the slightest clue as to the command's meaning. Regarding @ARGV I would look for 2 or more occurrences of either \$ARGV\[ or shift; outside of any sub or shift @ARGV; and then recommend. That's no good. Whether you're doing it "correctly" or not, all you have to do with `@ARGV` is loop over it. The number of times `@ARGV` is referenced (i.e. how you loop over it) does not indicate how its contents are used at all.	[reply] [d/l] [select]
Re^4: The maybe it is better written this way tool by szabgab (Priest) on Dec 02, 2009 at 20:20 UTC
Re^5: The maybe it is better written this way tool by ikegami (Patriarch) on Dec 02, 2009 at 23:19 UTC
Re: The maybe it is better written this way tool by educated_foo (Vicar) on Nov 14, 2009 at 23:09 UTC
This same urge brought us clippy. I don't think anyone has figured out how to do this in a non-annoying way.	[reply]
Re^2: The maybe it is better written this way tool by mpeg4codec (Pilgrim) on Nov 16, 2009 at 01:16 UTC
Spelling and grammar check are along the same lines as these and nobody complains about them being annoying. The difference: they don't jump out at you uninvited. Make it a menu option: Comment on Coding Style. Alternatively, do the squiggly red/green underline trick found in popular word processors.	[reply]
Re^3: The maybe it is better written this way tool by educated_foo (Vicar) on Nov 17, 2009 at 06:59 UTC
Right. Spell check is easy, rarely wrong, and east to correct via a "learn this word" option. Grammar checking is hard, often wrong, and hard to correct. That's why I either turn it off or learn that green squiggles mean "no problem." Unlike spelling, coding style is impossible to codify; it is basically "whatever the current Kool Kidz say," said Kidz are different every day, and they are almost never able to specify -- much less justify -- what they mean. Either the problem's easy (spelling), corrections are ignored (squiggles), or the "solution" is an annoyance (clippy).	[reply]
Re: The maybe it is better written this way tool by Anonymous Monk on Nov 14, 2009 at 21:01 UTC
Change `foreach my $line ( <FILE> ) {` to `while ( my $line = <FILE> ) {`	[reply] [d/l] [select]
Re: The maybe it is better written this way tool by Anonymous Monk on Nov 14, 2009 at 21:08 UTC
`$#array` refers to an index value into the array `@array`. It should never be used to determine the number of elements of `@array`. For example: `if ( $#ARGV < 2 ) {` [download] Should be written as: `if ( @ARGV < 3 ) {` [download]	[reply] [d/l] [select]
Re^2: The maybe it is better written this way tool by ikegami (Patriarch) on Nov 14, 2009 at 21:55 UTC
[`$#array`] should never be used to determine the number of elements of @array. eh? Why not?	[reply] [d/l]
Re^3: The maybe it is better written this way tool by zwon (Abbot) on Nov 14, 2009 at 22:11 UTC
I think Anonymous Monk make hints about `$[` `$[ = 10; my @arr = ('a', 'b'); use Data::Dumper; warn Dumper [ $#arr, scalar @arr ]; __END__ $VAR1 = [ '11', 2 ];` [download] Actually I think it would be better if this tool will suggest not to change `$[` in the first place.	[reply] [d/l] [select]
Re^4: The maybe it is better written this way tool by ikegami (Patriarch) on Nov 14, 2009 at 22:18 UTC
Re^3: The maybe it is better written this way tool by Anonymous Monk on Nov 15, 2009 at 00:19 UTC
If we want the fifth element of an array why don't we say: `$array[ 5 ]` [download] Isn't that DWIM? Why does `@array` in scalar context evaluate to the number of elements? Why not just use `$#array + 1` and have an array in scalar context return the first element, or the sum of all the elements, or the total length of all the elements?	[reply] [d/l] [select]
Re^4: The maybe it is better written this way tool by ikegami (Patriarch) on Nov 15, 2009 at 01:24 UTC
Re^5: The maybe it is better written this way tool by Anonymous Monk on Nov 15, 2009 at 03:52 UTC
Some notes below your chosen depth have not been shown here
Re: The maybe it is better written this way tool by Anonymous Monk on Nov 14, 2009 at 20:58 UTC
Change `split / /, $var` to `split ' ', $var`	[reply] [d/l] [select]
Re^2: The maybe it is better written this way tool by ikegami (Patriarch) on Nov 14, 2009 at 22:11 UTC
On the same note, `split '...'` is more readable as `split /.../`, since the first arg of `split` is a regex pattern. (`split ' '` excepted, of course.) We often get questions about `split '\|'`.	[reply] [d/l] [select]
Re^3: The maybe it is better written this way tool by LanX (Saint) on Nov 17, 2009 at 23:37 UTC
> On the same note, split '...' is more readable as split /.../, since the first arg of split is a regex pattern. (split ' ' excepted, of course.) We often get questions about split '\|'. I don't understand, don't you mean it the other way round? IMHO since /.../ clearly shows that it's a regex it should better always be written like that. '...' misleadingly looks like a simple string. Cheers Rolf	[reply]
Re^4: The maybe it is better written this way tool by ikegami (Patriarch) on Nov 18, 2009 at 00:01 UTC
Re^5: The maybe it is better written this way tool by LanX (Saint) on Nov 18, 2009 at 00:52 UTC