Beefy Boxes and Bandwidth Generously Provided by pair Networks
We don't bite newbies here... much
 
PerlMonks  

Re^3: inheritance: constructors

by Athanasius (Monsignor)
on Dec 01, 2012 at 03:33 UTC ( #1006566=note: print w/ replies, xml ) Need Help??


in reply to Re^2: inheritance: constructors
in thread inheritance: constructors

I couldn’t agree more. One of my pet abominations is code like this:

if ($i != $j) { # not equal } else { # not not equal -- huh? }

which can so easily be recast as:

if ($i == $j) { # equal } else { # not equal }

So, let’s propose a general style guideline:

Thou shalt not code in double negatives.

For an amusing commentary on this theme, see Re^2: Can't remove directory-Permission denied (knot).

And for the one legitimate exception to the rule (that I can think of offhand), see Secret Perl Operators: the boolean list squash operator, x!!.

Athanasius <°(((><contra mundum


Comment on Re^3: inheritance: constructors
Select or Download Code
Re^4: inheritance: constructors
by jdporter (Canon) on Dec 01, 2012 at 04:12 UTC

    Sorry, I'm not buying. This is not ok? --

    if ( $value != $expected_value ) { $errorcount++; die new Exception msg => "argh!"; } else { # continue with the happy path }

    But this is ok? --

    if ( $value != $expected_value ) { $errorcount++; die new Exception msg => "argh!"; } # continue with the happy path

    no, no, no.

    I reckon we are the only monastery ever to have a dungeon stuffed with 16,000 zombies.

      Well, in

      if (...) { die ...; } else { # continue with the happy path }

      the provision of an else block is redundant, since it suggests (misleadingly) that the code can somehow die and yet still resume execution following the else — which it can’t.

      For this case, I would prefer:

      unless ($value == $expected_value) { $errorcount++; die new Exception msg => "argh!"; } # continue with the happy path

      which reads more naturally (to my ear). (Note that the original objection was not to unless, but to unless ... else.)

      Update: Nice use of the <abbr> tag, BTW — I’ve learnt something new, could come in handy.  :-)

      Athanasius <°(((><contra mundum

        Right. Which means your "rule" is inconsistent. What happens inside any of the blocks shouldn't matter.

        I don't buy the argument that "unless" "reads better" than "if !" (nor that "reads better" is always a good criteria for evaluating code, especially not "reads more like English prose").

        I do buy the argument that making the negation more subtle (by using "unless" instead of "if !") also means that you increase the odds of mentally dropping the negation and either introducing a bug or wasting too much time being confused (having experienced this and having seen others do it and having seen quite a few bugs make it to Production that were introduced via 'unless').

        Another unfortunate side-effect of that "subtle 'not'" that I was a bit surprised and dismayed to repeatedly see in real code recently, was constructs like:

        unless( $a ne $b ) {

        and worse. The 'not' is subtle so once you start using 'unless' all over the place you'll likely start introducing double negatives.

        So I have a strong preference for:

        if( $a eq $b ) { # or if( $a ne $b ) {

        over anything using 'unless'.

        Which brings me to a subject we were discussing this week at work. I've heard (or read) a lot of people agreeing that "of course" you should not use 'unless' with complex logical expressions. But that seems to me to be the one place where I can see 'unless' having some value:

        croak( "You must pass foo() a non-empty array ref" ) unless $arg && 'ARRAY' eq ref($arg) && 0 < @$arg;

        Directly turning that 'unless' into an 'if' gives us:

        croak( "You must pass foo() a non-empty array ref" ) if ! $arg || 'ARRAY' ne ref($arg) || 0 == @$arg;

        Now, I don't personally find that particular 'if' obviously worse than the prior 'unless'. But I acknowledge that it is more complex and can see it being harder or slower to parse for many. So I'm somewhat reluctant to make such a transformation when refactoring code.

        And it can get "worse":

        croak( "You must pass foo() a non-empty array ref or hash ref" ) unless 'ARRAY' eq ref($arg) && 0 < @$arg || 'HASH' eq ref($arg) && 0 < keys %$arg;

        This would require adding parens, which can reduce readability considerably (not terrible in this example, though -- certainly more complex):

        croak( "You must pass foo() a non-empty array ref or hash ref" ) if( ( 'ARRAY' ne ref($arg) || 0 == @$arg ) && ( 'HASH' ne ref($arg) || 0 == keys %$arg ) );

        So, I'm more of the opinion that 'unless' should only be used in the case of complex logical expressions. That is, only for complex logical expressions where the 'unless' version is significantly simpler than the 'if' version.

        For the sake of completeness, I'll note that I often find value in removing the complex logical expression (which also removes any benefit to using 'unless', IMO). In the above example, if we get an error about "You must pass foo() a non-empty array ref or hash ref", then we have several possible cases to consider and this could add a step or two to debugging that might add significant time to the process. So it might be better more like:

        croak( "You didn't pass foo() a reference" ) if ! $arg || ! ref $arg; if( 'ARRAY' eq ref $arg ) { croak( "You passed foo() an ARRAY ref but it was empty" ) if 0 == @$arg; ... } elsif( 'HASH' eq ref $arg ) { croak( "You passed foo() a HASH ref but it was empty" ) if 0 == keys %$arg; ... } else { croak( "You passed foo() a ", ref($arg), " ref (must be ARRAY or HASH)", ); }

        That looks a lot more complex but in reality, it will likely eliminate duplication because your check for "is an ARRAY ref" needs to also be done where you decide what to actually do with the ref and this structure lets you do that check just once. That can also have benefits by keeping the error checking closer to the associated code so that deciding to handle an empty hash is more likely to have the error checking updated to match at the same time.

        I'm now considering if I should avoid the subtle 'not' in complex cases by using the low-precendence 'not' as in 'if not ...' to replace the 'unless'. I haven't tried that anywhere yet, so I can't really recommend it yet.

        Note that I've seen experienced programmers introduce real bugs that make it to Production because they used 'and', 'or', and/or 'not' in logical expressions (presumably because it "reads better") and were not always aware enough of some of the impacts of the extremely low precedence. So "if( not ... )" would end up being a special-case exception to our rule against using low-precedence logical operators in logical expressions.

        - tye        

        I would, in this case, be tempted to write
        die if ...; #happy path continues
        I might even be willing to accept a die unless if the positive version is easier to read than the negative one.
Re^4: inheritance: constructors
by Anonymous Monk on Jun 18, 2013 at 20:30 UTC

    You assume the order of the code blocks does not matter, too most ppl it does.

    Either: if ($i != $j) or: unless($i == $j)

    Chose one, these are the same. Your example changes much more than just removing a double negative.

Log In?
Username:
Password:

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

How do I use this? | Other CB clients
Other Users?
Others rifling through the Monastery: (9)
As of 2014-07-26 01:05 GMT
Sections?
Information?
Find Nodes?
Leftovers?
    Voting Booth?

    My favorite superfluous repetitious redundant duplicative phrase is:









    Results (175 votes), past polls