in reply to Burned by precedence rules

I like to write the dumbest and most readable code that will the job done, and only expand from there when there is a real need for it in the design.

sub done {
    my $self = shift; 

    if ( $self->foo or $self->bar ) {
        return 0;
    }

    return 1;
}

[download]

I've had many issues bite me in the tail, but precedence is rarely one of them. Then again, I have such a huge itch about readability that I had to fight off the urge to define TRUE and FALSE constants for this tiny slice of code. So take these thoughts with however many grains of salt you desire.

Replies are listed 'Best First'.
Re^2: Burned by precedence rules
by gwadej (Chaplain) on Dec 26, 2008 at 17:37 UTC

    I would urge you not to define constants for TRUE and FALSE when the language doesn't provide them.

    One of the nastiest bugs I ever had to track down was caused by someone defining these constants slightly differently than the way the language did. We were constantly surprised when !$is_bad, FALSE == $is_bad, and TRUE != $is_bad did not give the same answers.

    My favorite readability move for code like this would be to rename the methods (maybe is_done) to work better in conditionals.

    In your example, I have a bigger problem understanding why foo returning a true value means that we are not done. If, on the other hand, the method were named is_foo_running it would be easier to understand.

    Although I have fought the no magic literals fight for years, true and false are not where I prefer to fight.

    G. Wade
[reply]
[d/l]
[select]

      I disagree.

      The problem is not in defining 'true' and 'false' constants other than ones predefined by the language. The real problem is using any such Boolean constants in (in)equality expressions.

      One should never write anything like "== false" and one should rewrite any such code that one runs into (when practical). For one thing, it is needless complex, even redundant.

      Yes, defining 'true' and 'false' constants exacerbates this problem and the problem is somewhat mitigated when a language has a first-class Boolean data type (that can't be made to hold other than the two special values) -- which also means that the language likely defines its own symbolic representations for those two special values.

      But one can certainly sanely use 'true' and 'false' constants even in Perl. Just don't test for equality against Boolean values, especially against Boolean constants. And one should follow this practice even in languages where a real Boolean datatype mitigates the seriousness of such redundant constructs.

      - tye        

[reply]

        I'm sure i shouldnt say this, but more than once I have written 

        if ( !$x == !$y ) { ... }
if ( !$x != !$y ) { ... }

        [download]

        which I think is a fair exception to your rule. I kinda view ! and !! as "boolean constructors", and so long as both sides of your (in)equality are guaranteed to be a "boolean" obtained one of these constructors you can compare them.

        But I'm really just nit-picking. :-)

        ---
        $world=~s/war/peace/g

[reply]
[d/l]

        Even outlawing comparisons to the constants isn't enough.

        sub is_tested {
    return FALSE;
}

        [download]

        and then used as

          if( is_tested() ) {
     print "is tested.\n";
  }

        [download]

        and, by the way, FALSE is defined to be 2. This was the sort of thing that haunted me for days.

        I was using those expressions to describe the problem not the usage.

        G. Wade
[reply]
[d/l]
[select]

      I had exactly the same thought on the sub name and "false means true" issues, but thought I'd leave it alone for the moment. I do think the use of constants in one form or another is a valid approach, but you need to make sure it's used consistently throughout the entire project. That can be a challenge.

[reply]

In Section Meditations