in reply to RFC: Perl-Critic policy: ProhibitInlineSystemArgs
This is probably not the right place for this comment, but I just scanned the policies of the perlcritic and I have to say I do violently disagree with quite a few.
- ProhibitLvalueSubstr? Well, maybe.
- ProhibitSleepViaSelect? Agreed.
- ProhibitStringyEval? The summary is incorrect, but yeah, string eval is to be used with care.
- ProhibitUniversalCan? Well well well ... the $foo->can(...) is easier to read, but you may have to check first that the foo is actually a blessed refererence. Otherwise instead of a false value you get an exception/error.
- ProhibitUniversalIsa? Same thing.
- RequireBlockGrep? Why???
- RequireBlockMap? Double why???
- RequireGlobFunction? Why not $FH->readline() and $res = regexp_replace( '\d+', 'number', 'g')? What about Assign( $x, Plus( 5, Times( $y, 7)))?
- RequireSimpleSortBlock? Yeah, anything more complex is too hard for the reader. Use the scalar comma instead of the semicolon if you need to do a bit more.
- ProhibitExplicitISA? If you insist ...
- ProhibitOneArgBless? Agreed.
- ProhibitHardTabs? Hey, it's my business what do I use to format my code, as long as I'm consistent. Why would I type (or have my editor generate) four (or whatever's your favourite number) spaces when one tab is enough? You don't mix'em!
- ProhibitParensWithBuiltins? Severity 1? OK. I don't think it matters at all but ...
- ProhibitQuotedWordLists? Yeah and confuse newbies and people that have Perl as their tenth and seldom used language just to save a few characters. Why would anyone care to check for this?
- RequireTrailingCommas? OK, it's better to put it there in case you need to move things around, but ... is this worth testing?
- ProhibitCStyleForLoops? I think you meant "use for my $i (1..20) instead of for($i=0; $i<=20; $i++)" right? Am I supposed to use for my $i (map {$_ * 2} (0..10)) instead of for($i=0; $i<=20; $i+=2) as well? I'd agree that this for(my $i=0; $i <= $#arr; $i++) is a red flag and most likely a for my $item (@arr) would be preferable, but to outright "ban" the C style for loop?
- ProhibitCascadingIfElse? Well, sometimes there's no other way dude!
- ProhibitPostfixControls? Why don't we program in VBScript then? It's simple and totally dumbed down. You might like it. Postfix controls, be it "if", "for" or "while" often lead to code that more clearly captures the thought flow of the author, that is easier to read and stresses the important part of the sentence ... I mean ... statement.
- ProhibitUnlessBlocks? Kinda agreed.
- ProhibitUnreachableCode? Well, agreed. Unreachable code is quite common while debugging, but in production code it's a red flag. You either forgot to delete something or remove some debug stuff or simply misunderstood something
- ProhibitUntilBlocks? Perl has until()? I've never used it or seen it
- RequirePodAtEnd? Other people say, put the documentation next to the things being documented. I think they are right. Doesn't seem important to me.
- RequirePodSections? Agreed.
-
- ProhibitBacktickOperators? Why use something simple if you can use for example IPC::Open3, right?
- ProhibitBarewordFileHandles? Agreed.
-
- ProhibitOneArgSelect? OK
- ProhibitReadlineInForLoop? Agreed
- ProhibitTwoArgOpen? Agreed. Though q{<} looks horrible to me. See bellow.
- RequireBracedFileHandleWithPrint? Show me someone who does this!
- ProhibitFormats? I never tried them. They probably do have their place though. And I don't think the people that do use them need their hands held, they tend to know what they are doing.
- ProhibitTies? Ban the magic! She's a witch, she's a witch, burn her! (just came from Silent Hill) I don't see a reason to ban something that lets me use all the syntactic sugar Perl has for arrays and hashes with objects. If it helps me to write less and easier to read code. Would anyone want to have to rewrite the whole script once you find your hash doesn't fit in the memory?
- RequireRcsKeywords? Some people may want that, maybe.
- ProhibitAutomaticExportation? OK
- ProhibitEvilModules? OK
- ProhibitMultiplePackages? I wonder what proportion of well know and widely used CPAN modules would fail this. If I need a tiny little internal helper class I don't see why would I want to put it into a separate file.
- RequireBarewordIncludes? OK
- RequireEndWithOne? We can't have some fun, can we?
- RequireExplicitPackage? Agreed
- RequireVersionVar? Agreed
- ProhibitAmbiguousNames? $thisIsTheLastValueIveSeenInThisFourLinesLongLoopUsingTheDefaultVariable
- ProhibitMixedCaseSubs? I think there should be a ProhibitUnderscoredSubs available as well, I don't think we can all agree on this
- ProhibitMixedCaseVars? Same as above
- ProhibitDoubleSigils? I'd only prohibit double sigils if there is a [ or { following the variable name, I would not want to have to write push @{$arr_ref}, $foo. OTOH, @$foo[1,2,5] would left me wondering whether it really means what I think it does for a moment.
- RequireExtendedFormatting? I find the /x-ed regexps harder to read most of the time. Yeah if it's something long and complex then /x migth help, but ...
- RequireLineBoundaryMatching? Huh?
- ProhibitAmpersandSigils? Fine with me
- ProhibitBuiltinHomonyms? Agreed completely
- ProhibitExcessComplexity? OK
- ProhibitExplicitReturnUndef? Sure
- ProhibitSubroutinePrototypes? Agreed. most of the time
- ProtectPrivateSubs? OK
- RequireFinalReturn? Well ... if the subroutine is longer than one or two lines.
- ProhibitNoStrict and ProhibitNoWarnings? You might want to warn about this, but I'd think that if you do use strict/warnings and just turn them off for a moment then you probably know what you are doing.
- RequireUseStrict and RequireUseWarnings? Sure
- ProhibitConstantPragma? I know how to handle my constants thank you very much. Readonly scalars are not inlined so the optimizer has no way to make use of the fact they are constant. Might mean a lot for thing like print STDERR "..." if DEBUG; in a tight loop.
- ProhibitEmptyQuotes? What? Beg your pardon?? Why the heck would I want to use q{} in place of ''??? And severity 2????
- ProhibitEscapedCharacters? Well... does anyone know where do I find the table of those \N{whatever} things??? OTOH everyone knows what \x0D\x0A means. Besides I bet quite often there either is no name or you really want this specific byte, not whatever your OS thinks is \N{foo} at the moment.
- ProhibitInterpolationOfLiterals? OK. Though the only speed difference is during compilation AFAICT.
- ProhibitLeadingZeros? OK
- ProhibitMixedBooleanOperators? Agreed. though I wonder whether the policy really prohibits just the mixed use.
- ProhibitNoisyQuotes? What? Beg your pardon?? Use q{} to make them even noisier? q{,} instead of ','??? I mean, if I need to include quotes in the string, I'm all for switching to q{} or qq{} or even heredocs, but requiring the use of q{} for anything that's not purely alphanumeric?? What's noisy about comma?
- ProhibitVersionStrings? OK
- RequireInterpolationOfMetachars? I don't think I understand this one.
- RequireNumberSeparators? Don't care.
- RequireQuotedHeredocTerminator? Agreed
- RequireUpperCaseHeredocTerminator? Why would I want to restrict the terminator if I need to be sure it will not be present in the literal?
- ProhibitConditionalDeclarations? Right, this is not well defined so let's not do this.
- ProhibitLocalVars? Agreed
- ProhibitMatchVars? Agreed
- ProhibitPackageVars? Sometimes package vars are the right thing
- ProhibitPunctuationVars? And send most people searching the source code where did the $EVAL_ERROR variable came from. What's the "ENGLISH" for $!, $/ and $; ? Did you know what it was without consulting the docs?
- ProtectPrivateVars? OK
- RequireInitializationForLocalVars? my $data = do {local $/ = undef; <$FH>}? Well, if you insist.
It looks like I do not feel so strong about most as I thought at first, looks like the first several I looked at were the (IMHO) craziest, but anyway ... there are some rules I would rather people not follow. :-(
Re^2: RFC: Perl-Critic policy: ProhibitInlineSystemArgs
by jthalhammer (Friar) on Jul 02, 2006 at 08:05 UTC
|
For most of the Perl::Critic policies, you can find a complete explanation in Conway's book "Perl Best Practices". If you're still not convinced, then you can always disable them. And if you stongly believe that a particular Policy should not be followed, then you're welcome to write a contra-policy (for example: ProhibitUnderscoreSubs). Perl::Critic is all about encouraging consistency. You can make whatever rules you like -- Perl::Critic is just there to help you and your team stick to those rules.
-Jeff | [reply] [Watch: Dir/Any] |
Re^2: RFC: Perl-Critic policy: ProhibitInlineSystemArgs
by BrowserUk (Patriarch) on Jul 02, 2006 at 08:07 UTC
|
The real problem here is that it is very easy to create new laws, it is just very hard to create good ones, and almost impossible to repeal bad ones.
Things like PerlCritic and PBP have a habit of becoming defacto-standards, and like voting for political parties, you either vote for or against. You cannot vote for tax breaks and against cuts in health care, you gotta take the package. And for every good suggestion or two or three they contain, there also lurks a bad one. Or one that should only be applied under specific circumstances, or one that should only be applied until the programmer can justify not applying it on the basis of his/her knowledge of the underlying reasoning; or the nature of their code; or the naturee of the usage of their code--but defacto-standards do not get mandated by programmers with such knowledge.
Defacto-standards get applied as blanket requirements by PHBs, weak coders who want someone else to make their decisions and obsessive-compulsive paranoid pedants that once bitten through lack of knowledge or care, become twice shy about using their own faculties.
Examine what is said, not who speaks -- Silence betokens consent -- Love the truth but pardon error.
Lingua non convalesco, consenesco et abolesco. -- Rule 1 has a caveat! -- Who broke the cabal?
"Science is about questioning the status quo. Questioning authority".
In the absence of evidence, opinion is indistinguishable from prejudice.
| [reply] [Watch: Dir/Any] |
|
Defacto-standards get applied as blanket requirements by PHBs, weak coders who want someone else to make their decisions and obsessive-compulsive paranoid pedants that once bitten through lack of knowledge or care, become twice shy about using their own faculties.
All true.
But what about folk like me who find tools like Perl::Critic useful aids in approaching lumps of bad legacy code, who are (hopefully) bright enough to realise that a code smell is just a smell and not necessarily evil in all cases. Who can read through PBP and agree with some things and disagree with others.
Are we supposed to throw away useful tools like Perl::Critic because some idiots misuse them?
In this particular instance it's been my experience - which obviously differs from yours - that people are always throwing unclean user input into single-string calls to system. I'd love a tool that helps me find these instances more quickly.
| [reply] [Watch: Dir/Any] |
|
Then you are the perfect audience for such a tool, and you will derive great benefit from it so long as you are in a position, (of sufficient authority), to make personal judgements about which rules to apply and when.
But just as many experienced drivers are capable of deciding that 55/65/70 mph is too fast in fog/driving rain/icy/high traffic volume conditions, they are also capable of deciding that it is unnecessarially slow on an empty highway, on a dry, sunny, Sunday morning, but the power-that-be are most unlikely to take a favourable view of them making the latter judgement. Unfortunately, the law is an ass simply because the is no way to enshrine the concept of good judgement into it.
In Germany on certain sections of autobahn, and I believe in Montana on some highways, there is the concept of unlimited speed. If the right vehicle is being driven in the right conditions, in the right way, then there is no set limit over which you can be prosecuted for speeding per se. However, you can be prosecuted for the full range of other road offences--undue care; dangerous driving; endangering other road users; under the various national guises.
So, if you are sure that the vehicle your are in is capable, and the road conditions permit, and you are sure that you will not encounter a policeman who is disgruntled because his application to be on duty at the stadium where the national team are playing Brazil was turned down, then by all means put your foot down. 'scuse me while I let my mind wander.
The point is that you may well be in a position of sufficient authority that you can make appropriate judgements, but for most programmers, most of the time, such rules tend to become black and white enforcements by people who do not understand the reasons underlying them. Second generation management that inherit a flexible system but decide rigid enforcement will better cover their deriars.
Examine what is said, not who speaks -- Silence betokens consent -- Love the truth but pardon error.
Lingua non convalesco, consenesco et abolesco. -- Rule 1 has a caveat! -- Who broke the cabal?
"Science is about questioning the status quo. Questioning authority".
In the absence of evidence, opinion is indistinguishable from prejudice.
| [reply] [Watch: Dir/Any] |
|
|
|
|
|
The real problem here is that it is very easy to create new laws, it is just very hard to create good ones, and almost impossible to repeal bad ones.
I think you are stretching this analogy to government farther than it will go. Many of the Perl::Critic rules are very useful and it's absolutely trivial to not use ones you don't like.
You cannot vote for tax breaks and against cuts in health care, you gotta take the package.
At YAPC::NA in Chicago last week, nearly every presenter who mentioned PBP said "except for a, b, and c, which I think are lame." The Perl::Critic presentation emphasized how to choose which tests to use and turn it off for sections where you need to. People are not having any trouble rejecting parts that they don't like.
| [reply] [Watch: Dir/Any] |
|
To clarify, I think the idea of Perl::Critic is a very good one, but I also think that the rules should be written to encapsulate as much as possible the finer details of when something is good or bad.
With respect to the all or nothing nature of the beast, as I explained to adrianh, whilst the programmer is personally responsible for deciding which rules to adhere to and which to eshew, it is a very useful tool in his toolkit.
The problem arises when blanket decisions are taken on high and enforced without individual judgement upon programmers by PHBs. If you have never had to comply with assanine coding standards, you will not understand my ire here. If you have, you'll know that it can completely ruin a working environment.
The problem comes when one member of staff starts using and recommends (for example), the PBP set of rules, and a non-coder management type takes up the cause and decides that all code, and all staff must code to the full set unselectively. It happens.
Once such sets of rules are set inplace, it becomes hell's own job to change them--especially from the outside or lower ranks.
Examine what is said, not who speaks -- Silence betokens consent -- Love the truth but pardon error.
Lingua non convalesco, consenesco et abolesco. -- Rule 1 has a caveat! -- Who broke the cabal?
"Science is about questioning the status quo. Questioning authority".
In the absence of evidence, opinion is indistinguishable from prejudice.
| [reply] [Watch: Dir/Any] |
Re^2: RFC: Perl-Critic policy: ProhibitInlineSystemArgs
by Herkum (Parson) on Jul 04, 2006 at 03:23 UTC
|
Jenda,
Please don't hold back, tell us what you really think! :) OK, enough with giving you are hard time...
What I have found for Perl::Critic is that it is a good developer testing module. It seems you have some strong opinions about numerous rules for Perl::Critic and that is OK. However, Perl::Critic addresses two major concerns that I consider more important than the actual rules
- Personal preference: It is easy enough to disable the rules you don't like.
- Consistency: If there are two programmers working together, you would like them to be able to have some sort of consistency in the work between them.
Also since Perl::Critic is a developer testing module rather than a required distribution module, there is no reason that it should be an issue when writing your code.
It is a tool that should make you happier and help, not be an opinion that is forced on you.
| [reply] [Watch: Dir/Any] |
|
|