Boolean logic and intentions

Boolean logic

I’ve seen quite a few blog and forum posts about how developers “abuse” boolean logic.
The most common of these examples would probably be something like:

bool userIsAuthenticated = Foo || Bar;
if (userIsAuthenticated)
{
   return true;
}
else
{
   return false;
}

And people are arguing about how bad that is and that it can be shortened down to:

bool userIsAuthenticated = Foo || Bar;
return userIsAuthenticated;

And then the next guy comes along and says it can be shortened down even more to:

return Foo || Bar;

And the general consensus is that the developer who wrote the first piece of code don’t know how boolean logic works and should be fired etc, the standard internet forum user attitude.

I personally find this somewhat strange, because I don’t think the first version is that bad.
Sure, it is bloated and much bigger, but is that all that counts?

I’m not into the “less is more” mindset, I’m well aware that allot of code can be shrunken into short incomprehensible nested and recursive expressions.

I don’t want “small” code, I want readable code that clearly communicates the intentions of the developer.

I find the first version very explicit, it tells the reader the exact intentions of the developer.
You can easily add comments to that kind of structure.
And you can also set breakpoints on the two return clauses, making life easier while debugging.

Don’t get me wrong, I would most likely go for the 2nd version in most cases;
Do the assignment on one line and then return that result.
But in some special scenarios I might go for the first version, simply because it communicates the intentions better.

The first version also opens up ways for us to add alternative flows.
e.g. Special logging messages when the result is true.

And I have to say that the same goes for:

if ( Foo == false )

It might be bloated, but the intention is very clear here.
The code could be shorter, but it isn’t, just because I want you to see my intentions.
So, Am I crazy?
Or do you also think that verbose code sometimes express the intentions better?

11 thoughts on “Boolean logic and intentions”

  1. I have to disagree with you. In this example, there is a very simple way to communicate your intentions: useful function names. In this case, your code snippet becomes:
    bool userIsAuthenticated(){
    return Foo || Bar;
    }
    Which leaves no doubt as to what is happening. Using your preferred way adds no information at all:
    bool userIsAuthenticated(){
    bool userIsAuthenticated = Foo || Bar;
    return userIsAuthenticated;
    }
    It just adds another line to make an error in, another line that the user has to parse in order to understand your program. And of course, this is especialy true for the first example. If I encounter such I thing, it would take extra time to first of all read it, and then trying to figure out if there is a reason for using such convoluted code.

    Your final 2 questions are a False Dillema, a perl golfing line of code does not express the intentions as well as a somewhat more verbose version. But that doesn’t mean I don’t think you’re crazy ;)

  2. I sort of agree. Readability is more important than counting lines.

    I think the last version is fine as long as it is wrapped up in a well named method:

    public bool IsUserAuthenticated()
    {
    return Foo || Bar;
    }

    Breaking code up into smaller methods is having your cake and eating it to. It is short and clear with the added bonus of being reusable.

    My 2c.

  3. I agree that code should be reflect the intentions of the developer and the 3rd snippet clearly doesn’t do that: the reader of the code has to assume that Foo||Bar is ‘authenticated’ which is likely not going to happen unless surrounding code gives the line context (see below)

    Though I disagree with the if statement in the first snippet. Readable code is also code that’s is compact and to the point: a screen can only contain a given amount of lines. So I find the second snippet best. However, would this be in a method ‘CheckIfAuthenticated(…)’, then I think snippet three is better, as the method name gives the code context (as it should).

  4. I agree with previous comments. The first version is very redundant if the method’s name is clear : every information is told two time.

    what’s more leaving space for potential comments/log is a useless loss of time as it is speculative. This will be done easily when another developer will really add a comment or a log. Meanwhile it decreases the readability for speculative reasons.

  5. As I stated in the post,
    I would go for the 2nd version most often.

    But the first version does allow things like breakpoints on the different return clauses.

    Let’s say you have a loop that loops for 1000 iterations untill the value becomes true.
    And you might onle want to debug when that happens.

    You can ofcourse do this with conditional breakpoints, but I’m a person of habit and just like to click the sidebar for my breakpoints.

    So I’m absolutely not saying that I _recomend_ version 1.
    Just that can be useful in some conditions and that I really don’t cry my eyes out when I see such code.

  6. You make a great point here: Readability goes before reducing the number of lines. I think the number one reason we should reduce the lines of code is to make it easier to understand the code. But if reducing the lines of code reduces the readability, then the effect is lost.

    What is more readable is somewhat subjective, as the comments so far also reveal. Here’s how I would implement it:

    bool userIsAuthenticated(){
    return Foo || Bar;
    }

    I totally agree with Maarten when he talks about useful function names.

    When it comes to:

    if ( Foo == false )

    I see your point, and it is not as stupid as some would say it is, but I still think that a person should know the programming language well enough to understand this as easy:

    if (!Foo)

  7. Readability is almost certainly the only important thing in your example. Fewer lines of code in this example is meaningless.

    Here is some code I knocked up to test this:

    public bool Test1()
    {
    bool userIsAuthenticated = GetRandomTruth();
    if (userIsAuthenticated)
    {
    return true;
    }
    else
    {
    return false;
    }
    }

    public bool Test2()
    {
    bool userIsAuthenticated = GetRandomTruth();
    return userIsAuthenticated;
    }

    public bool Test3()
    {
    return GetRandomTruth();
    }

    Now, lets look at the complied code using RedGate Reflector

    public bool Test1()
    {
    return this.GetRandomTruth();
    }

    public bool Test2()
    {
    return this.GetRandomTruth();
    }

    public bool Test3()
    {
    return this.GetRandomTruth();
    }

    The compiler will condense this for you, which ever way you code it. You get no advantage in writing less code.

    Personally, I would write it as example 2 or possibly 3, but there definately isn’t anything wrong in version 1

  8. “if ( flag == false )” is correct if that is the way you also speak: “if it rains tomorrow is false, then I’ll go for a walk”. Otherwise not, in my opinion. I also believe that it shows a lack of understanding of what a boolean actually is. Pepole who writes this way often get confused by a statement like “flag = x < 0;”. At least that’s my exerience…

  9. I think the first way is definitely not good. Ten lines of code to do what is in fact a single function (in the sense of ‘return either (foo, bar)’ ).
    I would normally not consider defining a function just for the third way even as the intention should be clear with a little context (although something like userIsAuthenticated, yes, since it’s a cross-context concept the means for which could be subject to change).

    Neither do I agree with ‘flag == false’, since it suggests that ‘flag == true’ is also valid.

    That is, I don’t mind less competent programmers doing what they need, as long as the distinction is made between what they’re suitable for and what is actually technical and of consequence.

    Whatever you assign concerned programmers, don’t drive them to compensate for their lack of understanding/logic with *imaginatiooon* (DailyWTF’s SODs).

Leave a Reply

Fill in your details below or click an icon to log in:

WordPress.com Logo

You are commenting using your WordPress.com account. Log Out / Change )

Twitter picture

You are commenting using your Twitter account. Log Out / Change )

Facebook photo

You are commenting using your Facebook account. Log Out / Change )

Google+ photo

You are commenting using your Google+ account. Log Out / Change )

Connecting to %s