Beefy Boxes and Bandwidth Generously Provided by pair Networks
Perl-Sensitive Sunglasses
 
PerlMonks  

perlcritic and OO Perl (including Moose) idioms

by boftx (Deacon)
on Sep 20, 2013 at 01:54 UTC ( #1054930=perlquestion: print w/ replies, xml ) Need Help??
boftx has asked for the wisdom of the Perl Monks concerning the following question:

The shop I am at has decided to try using perlcritic as one part of establishing code standards. Fair enough. But running it on some existing code reveals that perlcritic complains about things that are either a) what I consider to be a deeply ingrained perl idiom, or b) fully legal, in fact required, aspects of Moose.

Here is a skeleton Moose module that demonstrates both of the conditions I have mentioned above (perlcritic output is at the end of the module.)

package CriticTest; use Moose; our $VERSION = '1.01'; has 'needs_build' => ( isa => 'Int', is => 'rw', lazy => 1, builder => '_build_needs_build', ); sub _build_needs_build { my $self = shift; my %args = @_; return $self; } 1; __END__ [~/perl/test]$ perlcritic -1 CriticTest.pm Private subroutine/method '_build_needs_build' declared but not used a +t line 14, column 1. Eliminate dead code. (Severity: 3) Always unpack @_ first at line 14, column 1. See page 178 of PBP. (S +everity: 4)

I know that I could pepper my code with ## nocritic tags, but that seems particularly wrong for what is (IMHO) perfectly acceptable Perl. I also know that I could disable the rule that is complaining about unpacking @_, but that rule is quite desirable when dealing with non-OOP sub-routines.

Can anyone point me in the right direction to deal with what I think is undesirable behavior by perlcritic?

Comment on perlcritic and OO Perl (including Moose) idioms
Select or Download Code
Re: perlcritic and OO Perl (including Moose) idioms
by Athanasius (Monsignor) on Sep 20, 2013 at 02:13 UTC

      That was the first thing we did to verify what was being complained about. Yes, that works, but ...

      1) we really don't want to replace my $self = shift; in hundreds, if not thousands, of places in our code base.

      2) my $self = shift; is what I would consider a very popular idiom that has been in use since (I suspect) many of us first read about OO Perl and how it relates to farm animals. (BTW, where can I find a copy of that very informative original tutorial?)

      3) Most importantly, it does nothing to address the following code that allows for rather flexible arg passing (not saying if this is good or bad, just that this concept is pretty standard, too.):

      my $self = shift; my %args = @_ == 1 ? ( period => shift ) : @_;

      I appreciate the response, but unfortunately that has already been viewed as a less-than-optimal resolution.

Re: perlcritic and OO Perl (including Moose) idioms
by Anonymous Monk on Sep 20, 2013 at 02:45 UTC

      Thanks! That will help for that, and gives me a few ideas about where else I can look.

        Hmm, if I were in your boat I might write something like Perl::Critic::Policy::Moose::BuilderNamingConvention -- ensures proper prefix/sufix for has(...builder...)

        But with a better name :)

Re: perlcritic and OO Perl (including Moose) idioms (perlcritic is dead/dumb, perlcriticrc for moose)
by Anonymous Monk on Sep 20, 2013 at 03:01 UTC

    Can anyone point me in the right direction to deal with what I think is undesirable behavior by perlcritic?

    Stop using perlcritic as if its alive, its dead. perlcritic is dumb, it can't make choices for you, you have to read the docs and make your choices, and update your configuration

    Read https://metacpan.org/module/Perl::Critic::Policy::Subroutines::ProhibitUnusedPrivateSubroutines, update your critic configuration file, the end :)

    See also Perl::Critic::Moose for extra criticisms to install/configure/complainabout :)

    You might try finding an existing "perlcriticrc for moose" , not unlike Perl::Critic::Dynamic::Moose

    If I used perlcritic or moose more I might know more about this, but currently I'm satisfied with mentally ignoring any rules that don't make sense for me on the rare occasion I use perlcritic

      If I were in a position to establish the rules I would not bother with perlcritic but instead rely much more heavily on code reviews. But ...

      Unfortunately, much of this will be a team decision, and I already know that the team is going to want to use perlcritic simply because it will automate some things (yeah, right.) In the meantime, I am looking for tips such as what documentation I really need to look at, and how others have addressed what I think are common problems.

      Thanks for the response, it was helpful.

      Update: changed title back to what I had originally.

        ah, to work on teams ; to work at all :)
        "Unfortunately, much of this will be a team decision, and I already know that the team is going to want to use perlcritic simply because it will automate some things (yeah, right.) In the meantime, I am looking for tips such as what documentation I really need to look at, and how others have addressed what I think are common problems."

        I'd suggest that you get your team to read the first section of the Preface to "Perl Best Practices" and note how many times the words "guidelines" and "suggestions" are used. Ensure they've read the the first sentence of the third paragraph: "This book doesn't doesn't try to offer the one true universal and unequivocal set of best practices.". Show them the publishing date (2005) and make certain they understand this is eight years old.

        All too often I've come across statements like "We've got to code it that way because PBP says so." Your team really do need to understand that these are guidelines and suggestions, not rules and regulations. I think once you get that message across, you'll have a much easier time of getting a concensus on a configuration which is useful in that it identifies code which could be improved, as opposed to one that blindly follows every PBP dot-point.

        Just so you know, I like a fair bit of the advice in PBP: parts I follow, other parts I don't. I didn't want this to come across as a PBP bashing exercise on my part. :-)

        -- Ken

Log In?
Username:
Password:

What's my password?
Create A New User
Node Status?
node history
Node Type: perlquestion [id://1054930]
Approved by Athanasius
help
Chatterbox?
and the web crawler heard nothing...

How do I use this? | Other CB clients
Other Users?
Others scrutinizing the Monastery: (12)
As of 2014-12-17 20:48 GMT
Sections?
Information?
Find Nodes?
Leftovers?
    Voting Booth?

    Is guessing a good strategy for surviving in the IT business?





    Results (33 votes), past polls