Used as a to-do list specifically for what you want to remember to do, and adapted to your particular needs, Perl::Critic is pretty useful.

Sorry. But that "you can configure it to your needs" argument simply doesn't hold water. Let's start with the extremes:

1. The experienced. Perler.

   When PC (thos initials say it all :) first came around, I ran it out-of-the-box against several scripts and modules -- some my own; some by authors I respect and trust -- and it just bugged me intensely.

   It whinged and whined and cried wolf about every other line of code. The only time I have ever seen so many warning messages was when I tried to compile my first Pascal program and mistakenly had used {}'s instead of BEGIN/END. The volume of output (from PC) was ludicrous; given the scripts and modules were all tested, well-exercised, thoroughly debugged code.

   First, I tried to # no perlcritic all the failing lines in one script of about 20 lines that had probably taken me 15 or 20 minutes to write, it took nearly an hour to understand what each message was meant to be warning me of -- there were a few warnings that without the benefit of PBP; I simply never did understand. The result was that the script almost doubled in size, and became unreadable because of all the garbage non-comments.

   So then I tried to evolve one of the modules to pass without disabling. I gave up on the script and the first two modules; because they simply could not be made complient and still work. That took almost two hours and the result wasn't like Java. It was worse. it over doubled in size because of all the useful idioms that had to be rewritten into prozaic generic syntac. And it ran substantially more slowly. But worse, with twice as many lines, it almost certainly contained twice as many bugs as a result. It was definitely twice as hard to understand (more lines; more to read and understand, for an experienced Perler), and therefore harder to maintain.

   So, to the idea that I could disable those warnings that I disliked. I did. I simply stopped using it. There's more about that further down...

2. The novice.

   The novice starts using PC up front; and discovers that every line of code he attempts to copy or adapt from any Perl books he has access to -- from mastering Perl to Perl in a Nutshell to Perl for Dummies and everything in between -- won't pass PC's default settings.

   But how can he configure it? He doesn't have the experience to know which warnings he should heed and which to ignore.

   Beginner's have enough trouble with warnings; if they get around to adding it, as often as not they end up commenting it out. With PC, even if they copy the literature slavishly, they still get whinged at for doing the wrong thing. Talk about discouraging.

... If I disable everything; what good is PC doing for me? It isn't But when I use the types of things it warns ne about; I do so with consideration at the time; what point is there to having it remind me?

For example: I purposely choose to use bareword file handles in top level scripts. I do this on the basis that different things should look different; and a top level script *OWNS* the global namespace so there is no conflict as modules should *never* mess with globals. And in modules -- complying with that rule -- I always use lexical file handles.

So now I would need two configurations for PC. And then another for another (personal) rule; and another; and another...SO then the game is deciding which of those configurations I should run against this particular script or module. But that is pointless because I already made my decision when I wrote the code.

And that's the second argument against PC. It does static analysis, after-the-fact. I do my analysis dynamically, as I write the code. Anything it chooses to tell me about, I've already made my decision about. (Or I don't want to hear it in the first place!)

And finally, it is completely possible to write totally unintelligible, crap, wrong and dangerous code that no amount of PC's static analysis will ever detect. Which means it still comes down to testing to determine whether the code does what it is meant to do or not. And once I've done my testing and concluded it works; I couldn't give two monkey's hoots whether some anal retentive, failed Perler, wannabe Java coder thinks the way I've achieved that is sliced bread or moldy goat's cheese.

Passing my tests won't guarantee that it is 100% bug free; but if it fails in use, it won't be down to anything that PC would have told me about.

Perl::Critic's power is not in slavish devotion to its default rules, its in its ability to be configured, its robotic ability to run the same checks again and again, its ability to communicate your style to the rest of your project, and its ability to teach new styles.

Configuration: Shutting up Perl::Critic with #nocritic is not how you do it. Like any other system that gives you false warnings, you will eventually get frustrated playing whack-a-mole and start ignoring them. There is a .perlcriticrc which has per project and per user configuration. You can turn rules on, turn rules off, change their severity and generally configure them. Here's the .perlcritic from Test-Simple. Perl::Critic has to pick something for its defaults, they're never going to be acceptable to everyone (though some of them are whoppers), accept that and put a little effort into configuring it.

Repeatability: This leads to the second strength of Perl::Critic, its your robot monkey. Like automated tests, the great strength of Perl::Critic is that it can perform the same mind numbingly complicated task over and over and over again and do it perfectly (or at least equally wrong) every time. While humans might be better than robots at making heuristic decisions, which really good style judgements require, humans are terrible at making repeated rote judgements. This is what Perl::Critic aims to do, the simple (and sometimes not so simple) judgements, across acres of code, again and again. This leaves the humans to do what they're best at. I've been programming Perl for 15 years and Perl::Critic still picks up mistakes. In fact it just found a case where I forgot to turn on strict and warnings, how about that.

Perl::Critic also acts as the pair over your shoulder saying "can't you do that better?" Is that variable declared with too broad a scope? Shouldn't you use three arg open? Do you really need to turn off strict refs for the whole subroutine? Couldn't you do that with a dynamic method call rather than an eval STRING? Did you forget an explicit return?

Communication: This leads into the next strength of Perl::Critic, if you put a .perlcriticrc in your project then everyone working on the project gets to use it. Along with a .perltidy, .perlcriticrc clearly communicates and checks the project style without having to read a big out of date wiki page or something. Whether your contributor is a novice who doesn't know what they're doing, or an expert that THINKS they know what they're doing, perlcritic and perltidy make it very easy for them to review their code and follow the standards.

As important, those standards are written down and must be followed by all. This avoids the common case where the project leader makes it up as they go along, and gets to commit sloppy code when they want to. The perlcritic policy chosen by the project might stink, but at least its a known quantity that everybody has to live with. Once its known, it can be debated and fixed in a useful manner. perlcritic and perltidy have kept me honest.

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. For newbies, bareword filehandles would be a very common one (considering the Perl documentation is loaded with them).

And it doesn't even have to be a human its teaching, it might be a very old code base. The ancient ExtUtils::MakeMaker code is now much better because we put some work into making it pass basic perlcritic. Auditing all that code by eye would have taken stupid amounts of time and I'm sure we'd have been much lazier about it.

I don't agree with many things Perl::Critic does by default, but its defaults are Perl::Critic's great weakness. Play to your tool's strengths instead. If you're devoting your energy to fighting Perl::Critic's default rules rather than configuring them, perhaps you have an issue with authority.

• 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.

---

With the rise and rise of 'Social' network sites: 'Computers are making people easier to use everyday'

Examine what is said, not who speaks -- Silence betokens consent -- Love the truth but pardon error.

"Science is about questioning the status quo. Questioning authority".

In the absence of evidence, opinion is indistinguishable from prejudice.

If you're arguing that the default choices are inappropriate and that the choices should be updated to reflect current Perl programming practices, I'd say that's generally correct (though there are, as I mentioned, a number of things you should, in general, do unless you are a highly-sophisticated programmer and know exactly what you are doing and what risks you are taking).

There is, however, a certain amount of handwaving in your argument against static analysis as a technique in itself. You say you "do my analysis dynamically". What does that mean? As far as I can tell, it means that you decide what to do as you write, determining as you write whether or not you will use a given construct.

For your bareword file handles example, does "I can safely use this bareword filehandle because I know I haven't used it anywhere else, and I'm sure that no module I've used uses this filehandle either because I've read them and made sure" capture the process?

Because that's still static textual analysis - you just happen to be using your brain and not a program to do it. If "dynamic analysis" means something else to you, it would be interesting to hear more about that.

If you're arguing that the default choices are inappropriate

Oh, the defaults are absolutely abominable, and when P::C first hit our consciousness, I argued fervently that a) they should be changed; b) that considerable thought (and preferably open discussion), be put into deciding what they should be changed to. I was ignored.

Now, having seen the way the overall project has evolved, my position now is far stronger: fixing the defaults would not fix the problems, which run far too deep for this to simply be a "configuration issue".

One problem is the matter of "quality control".

Anyone can write and upload a 'policy module'; and it seems that most of them get adopted without any serious consideration of their efficacy.

That is best exampled by tye's stark demonstration of your own misunderstanding with regard to the bareword filehandles issue. Do not feel bad about that, I think many others are in a similar position with regard to this issue. and many others. And there in a nutshell is the bigger problem.

A Perl::Critic policy for something, no matter how trivial:eg. trailing commas; capricious:eg. spell-checking POD; misunderstood:eg. bareword filehandles; or just out&out wrong:eg. requiring use base; becomes widely, and wrongly, legitimised

Witness the number of replies we see here that completely ignore the question asked and respond with edicts that "You must use lexical filehandles", even when they are unrelated to the question, and barewords are being used completely safely.

Configuration is not enough.

File-scope granularity for defaults is simply not enough. Programmers can (and should!) make their judgements on the appropriateness of particular constructs and idioms dynamically, ie. On a case-by-case basis.

Eg 1. If a subroutine requires stepping through an array by 2s, 5s, or 10s, a C-style for loop is the ideal construct; but like most perlers, I prefer to use a perlish for loop for most cases.

Eg 2. Using <> rather than <STDIN> may be the 'right thing to do' in most cases; but sometimes the requirement calls for reading from both in the same loop(s).

As a teaching aid, P::C does more harm than good.

Giving beginner's a bunch of arbitrary, capricious, pedantic and outright wrong "rules" to follow rather than encouraging them to experiment and learn by their mistakes; and learning to read the (amazingly comprehensive, if somewhat difficult) perl documentation carefully and thoroughly, will bread new generations of Perl programmers every bit as damaged as the archetypal VB programmers from yesteryear.

Monkey see, monkey do

This has nothing to do with snobbery; or being a "highly-sophisticated programmer"; it has everything to do with wanting to protect my favorite language and its future practitioners from the capricious, java-wannabe, idiots that think that they can save money on training by reducing Perl to some bland, flavorless, generic programming language.

We should not be entertaining, much less encouraging, people who think that while( ! eof() ){

is any clearer or easier to understand that until( eof() ) {.

Or that for my $line_out ( @array ) { say $line_out } is clearer, more understandable; or dog forbid, 'more correct' than say for @array.

---

With the rise and rise of 'Social' network sites: 'Computers are making people easier to use everyday'

Examine what is said, not who speaks -- Silence betokens consent -- Love the truth but pardon error.

"Science is about questioning the status quo. Questioning authority".

In the absence of evidence, opinion is indistinguishable from prejudice.

For your bareword file handles example, does "I can safely use this bareword filehandle because I know I haven't used it anywhere else, and I'm sure that no module I've used uses this filehandle either because I've read them and made sure" capture the process?

Don't be silly. A module using FOO as a bareword filehandle introduces no problems for me when I've used FOO as a bareword filehandle. Not even two modules each separately using FOO as a bareword filehandle causes a problem. *main::FOO is not the same as *Ancient::Module::FOO nor as *Modern::NonConformist::FOO.

And "I haven't used it anywhere else" isn't even good enough of a test. The test is much more like: I won't be calling this bit of code that opens bareword filehandle FOO again until after I no longer need access to the prior file I opened.

I'll be quite impressed if you can manage to get Perl::Critic to perform that test accurately.

- tye        

As for points 1 and 2, all I can say is: Amen!

( Much of the rest I agree with at least to some extent; but nothing to shout praise for nor to quibble over. :)

- tye        

thinks the way I've achieved that is sliced bread or moldy goat's cheese.

I think you meant to insert moldy goat turds cause moldy cheese is yum :p

Some of us do not consider sliced bread to be so great. Does that change your perspective?

---

With the rise and rise of 'Social' network sites: 'Computers are making people easier to use everyday'

Examine what is said, not who speaks -- Silence betokens consent -- Love the truth but pardon error.

"Science is about questioning the status quo. Questioning authority".

In the absence of evidence, opinion is indistinguishable from prejudice.