Beefy Boxes and Bandwidth Generously Provided by pair Networks
Just another Perl shrine
 
PerlMonks  

Re^4: The Most Essential Perl Development Tools Today

by schwern (Scribe)
on Jan 03, 2013 at 22:07 UTC ( [id://1011523]=note: print w/replies, xml ) Need Help??


in reply to Re^3: The Most Essential Perl Development Tools Today
in thread The Most Essential Perl Development Tools Today

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.

Replies are listed 'Best First'.
Re^5: The Most Essential Perl Development Tools Today
by BrowserUk (Patriarch) on Jan 04, 2013 at 17:12 UTC
    • 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% --

        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.

      While I mostly agree with this there is one use case for P::C that hasn't been mentioned yet, but I can see the potential benefit.

      Let's assume that I always use the two argument open. But that a few days ago, I made a new year's resolution to embrace three argument open as a matter of personal preference. I could use P::C with a highly stripped down configuration to remind myself not to use two argument open on all my code for the next couple of months until muscle memory takes over.

      So the use case is: reminding yourself about style guidelines that you have decided to follow, yet sometimes forget through laziness or force of habit. After a little while you probably start doing it automatically and can then abandon P::C. Or you don't, and decide to go back to your old way.

      perl -E'sub Monkey::do{say$_,for@_,do{($monkey=[caller(0)]->[3])=~s{::}{ }and$monkey}}"Monkey say"->Monkey::do'

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

      Your benchmark is rather bogus. The performance "hit" from the return statement is negligible. It's the prototype for no arguments with no return statement that gives a seeming big return (I'm guessing the interpreter is optimizing it to nearly a no-op).

      $ perl -E 'use Benchmark qw(timethese cmpthese); sub a() { 1 }; sub b( +) { return 1 }; cmpthese -1,{ a=>q[ a() for 1 .. 1000;], b=>q[b() for + 1 .. 1000;] };' Rate b a b 2694/s -- -88% a 23209/s 761% -- $ perl -E 'use Benchmark qw(timethese cmpthese); sub a { 1 }; sub b { +return 1 }; cmpthese -1,{ a=>q[ a() for 1 .. 1000;], b=>q[b() for 1 . +. 1000;] };' Rate b a b 2595/s -- -8% a 2823/s 9% --

      Is there a performance gain from not using return in this spot? Yes.

      Is it a premature micro-optimization which is unlikely to have anything but a negligible impact the vast majority of real world code? Yes.

      Would I consider this performance gain an invalid reason to skip a return statement on any code that hasn't been profiled and clearly shown to benefit from this micro-optimization? Yes.

        to skip a return statement

        Would I consider the flagrant waste of even 10% of cycles through the addition of a redundant and purposeless extra keyword, an affectation who's time has past. Emphatically so!.


        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.

        Is it a premature micro-optimization which is unlikely to have anything but a negligible impact the vast majority of real world code? Yes.

        No, its called style :)

        map, grep, sort, s{}{...}e don't allow you to use return, they force you to use implicit return

        explicit return isn't required in do/eval/sub

        return tells you about this

        so unless you're coding something recursive, something that requires short circuiting, most perlish perl programmers omit the explicit return, and use the implicit return

        If explicit return costs more (I'll take BrowserUk's word that it does), its a problem with the implementation, something for p5p to fix, same as they did for map in void context

Log In?
Username:
Password:

What's my password?
Create A New User
Domain Nodelet?
Node Status?
node history
Node Type: note [id://1011523]
help
Chatterbox?
and the web crawler heard nothing...

How do I use this?Last hourOther CB clients
Other Users?
Others cooling their heels in the Monastery: (6)
As of 2024-03-19 02:23 GMT
Sections?
Information?
Find Nodes?
Leftovers?
    Voting Booth?

    No recent polls found