• Configuration:

  Shutting up Perl::Critic with #nocritic is not how you do it.... There is a .perlcriticrc which has per project and per user configuration.

  That makes no sense at all.

  Firstly, if you have per-project and per-user configuration; which one takes priority? Which one should take priority? And why?

  I personally find the die ... unless <some required precondition>; idiom concise, eminently clear and extremely useful, so I would disable that wholly specious critique. But the project leader decides that he doesn't like it, so he enables it for all his projects. Which should take priority? And why?

  There is no right or wrong answer here because it is literally just a matter of opinion.

  Correctly written code that uses it has no additional risk of failure; its just someone's opinion that it might be confusing. It will never be confusing to me; or anyone familiar with the (full) language. So should I stop using it for the possibility of saving a newbie from learning the language he's using?

• Repeatability:

  its your robot monkey.

  But robot monkey's can only perform robot monkey tasks. What is the point of constantly re-running the same analysis on the same code? Am you likely to suddenly change your mind that the construct you wrote yesterday is no longer viable today or tomorrow?

  Perlcritic has one legitimate use: the analysis of existing code that is new to you. It can allow you to short cut your way into getting a feel for the style of code that you are now responsible for.

  But are you really going to risk breaking existing, tested, working code in order to make it compliant with your personal preferences or the latest fads? What will your boss/client/customer have to say if you do and it bring his business to its knees?

  1. Is that variable declared with too broad a scope?

     Show me an (real; not contrived) example where P::C detects a too wide scope that will actually cause harm. Then I'll consider that a valid argument and counter it.

  2. Shouldn't you use three arg open?

     I always use a 3-arg open except where I wish to benefit from the extended possibilities of the two or one-arg variants. But I certainly do not want to be bellyached at when I have explicitly chosen one of those alternatives. So the only "solution" is the #nocritic tagging, which you decry above.

     Or just kick P::C into touch.

  3. Do you really need to turn off strict refs for the whole subroutine?

     I always use the tightest scope possible for no strict .../no warning ... -- often as not a do{ no strict ...; ... } -- so another non-comment comment is required.

  4. Couldn't you do that with a dynamic method call rather than an eval STRING?

     Maybe, but it would probably result in more code -- to cater for all the possibilities -- and impose a greater startup cost -- compiling all the possibilities only 1 of which will ever be used in any given run.

     But why would I anyway? The only time string eval is any more dangerous that requireing a module (or just running a script) - both of which are just string eval wrapped over in a little file handling; is if untrusted input is incorporated into the string that is eval'd. So I don't do that.

  5. Did you forget an explicit return?

     Forget? No. Omit? Quite likely. Live with it. Cos nothing in this world is going to make me give up a 400% speed gain:

     [0] Perl> sub a(){ 1 }; sub b(){ return 1; };;
     [0] Perl> cmpthese -1,{ a=>q[ a() for 1 .. 1000;], b=>q[b() for 1 .. 1
     +000;] };;
          Rate    b    a
     b  4668/s   -- -80%
     a 23195/s 397%   --
     [download]

     Just in case some (Perl) newbie doesn't get that subroutines return the value of their last expression and need a keyword to tell them so. No way José.

• Communication:

  you put a .perlcriticrc in your project then everyone working on the project gets to use it.

  Sorry, but that's not communication; it is edict!.

  You seem to be suggesting that P::C can be used as a substitute for code reviews; but that is nonsense. We both know we could write any amount of code that would pass P::C's static scan, but be total crap. Inefficient, unmaintainable; logically corrupt and functionally useless, but bowing and scraping to the opinions of every jaundiced, wannabe Java programmer out there.

  The logic, efficiency and design of the smallest program can only (so far) be checked by the human eye of a seasoned programmer. That means proper code reviews. Nothing else will do.

  And once you accept that; P::C becomes entirely superfluous.

• Education:

  Finally, perlcritic teaches you things. Maybe it just constantly complains about a bad or out of date habit you have, or maybe it shows you something totally new. Three arg open is a great example, perlcritic has knocked that habit out of me.

  Again, we will have to agree to differ on that, because I don't think it does.

  • If you're a (Perl) newbie:

    it will simply force you into adopting a prosaic, generic coding style that avoids all the useful idioms and shortcuts that make Perl so productive, because you will not have the experience to challenge those bogus defaults you recognise; and never will.

    And you won't have the time or weight to challenge them; or whatever subset of them the project leader has imposed.

  • If you are an experienced Perler:

    You'll either disable the critiques that you've already decided you dislike; or never encounter the ones you already agree with.

    The only way you might learn is to throw it at old code written before your style had evolved. And then (re)consider every decision you made at the time. But unless that old code was due for a re-write anyway, then you'd be a fool to risk breaking working code it simply to make it pretty; or comply with the latest fads.

    And if it is due for a re-write; then you'd change the bad stuff anyway as a matter of course as you rewrote it.

  For newbies, bareword filehandles would be a very common one (considering the Perl documentation is loaded with them).

  And that goes to the very crux of my argument against P::C.

  In top-level script I explicitly prefer to use bareword filehandles. I won't reiterate my justifications here, but it is a considered and conscious choice that I believe benefits my code.

  However, I am fully aware that using them in modules it a complete no no. And I do not do so.

  And there are similar dichotomies with just about everything that P::C can (or could ever) flag. In some places; under some circumstances, the construct or practice is perfectly viable; and in others it is (IMO) not. And those dichotomies do not split along neat project or user or customer or even individual source file lines, they are dependent entirely upon where in the code they appear; and what the code is doing.

  And a static analyser like P::C can never even begin to take those kinds of factors into account. They require experience and knowledge of the project; the data; the runtime environment and the application audience. In short, they require judgement, and programs cannot yet supply that.

perhaps you have an issue with authority.

Ignoring the ad hominem implications; I'll respond seriously.

The only "issue" I have with authority; is that I understand its definition, which for the purposes of this discussion can be defined as:

authority refers to a claim of legitimacy, the justification and right to exercise that power.

Who is qualified to decide that statement modifiers are verbotten? Or that map must use the block form; but also must only contain a single, non $_-modifying statement?

Or that I should have to use some crass artificial construction like:for my $array_index ( map{ $_ *5 +3 } 0 .. int( $#array / 5 ) ) { ...}

Rather than for( my $i = 3; $i < @array; $i += 5 ) { ... }

Because, unless s/he can justify that with a logical reason that goes beyond their personal preference -- and that means way, way beyond the PBP justifictions for those things -- and the only way that could be done is to cite a real, non-contrived example of where those things produce bad code; not just give pause for thought to those new to Perl (or too lazy to get beyond the basics); and that person has greater relevant experience than I; I do not recognise that person as having any "authority".

As my boss -- or, at least notionally, my customer -- you may have the power to impose your edict upon me; but that is not authority.

I've said it once in this thread, but it bears saying again. P::C does not test anything. It will never find a bug that would not be found by proper testing.

The very best it can do (after code has been properly tested) is find potential bugs and potential misunderstandings. But applications are not improved by prematurely fixing potential bugs; and maintenance is not improved by avoiding training. It may be cheapened, but not improved.

And you cannot avoid testing by using P::C -- hopefully that statement doesn't need justifying -- so if you have to test anyway; and P::C cannot find bugs that testing won't; in the end all you have is a poor substitute for proper code reviews and a bean counting exercise.

Or to put it in financial terms; a pure cost exercise with no measurable benefit.

With a thought-through configuration used (once) on a new-to-you code base; P::C can be a useful tool for the individual programmer to find their way around. But as a repetitive pass over on-going development; it is little more than a feel-good statistic and an arse covering exercise. A waste of cycles and power and time.