Reputation: 480

Boolean types

During code review I discovered many places of our C# code that looks like this:

if(IsValid()) {
     return true;
}
else {
     return false;
}

or even "better":

return (IsValid()? true : false);

I always wondered why not just write the code like this:

return IsValid();

This is the way I would write this code. I ain't questioning the skills of the developers, but maybe trying to look into the developer's soul. Why would a developer favor more complex code and not a more simple and intuitive? Or maybe the reason is that it is hard to accept the Boolean type as the first-class citizen?

Upvotes: 2

Answers (10)

chaos

Reputation: 124297

Yes, you should do it as you say. These people are doing these overly verbose things because they first learned it that way, perhaps in CS 101, and it never occurs to them to go outside the space of what they know works to see if there is a better, easier way.

This does speak to their competence. Good programmers need to be a little more thoughtful and a lot less hidebound.

Upvotes: 7

Gavin Miller

Reputation: 43815

Distraction

I know it's occurred in my code before and I can trace that back to when I was interrupted or not paying attention while coding (I solely blame SO!)

Ignorance

Not knowing the better way to do it. We take it for granted that all programmers think logically, but that's not the case. Some coders are going purely on patterns of what they've seen before:

If (integerA == integerB) { //do special stuff }

//Given integer equality; boolean equality ought to look the same...
If (isValid() == true ) { //do special stuff }

Momentum

That's how someone has always done it and therefore that's how they continue to do it.

Upvotes: 0

dance2die

Reputation: 36905

First case is easier when you are debugging. When you step through your source, it is easier to find out what the return value is without opening up immediate window or run IsValid(); just to see the return value.

For the first and second case, developer might not be aware that s/he can simply do

return IsValid();

Lastly, a developer might be forced to use first or second syntax due to company policies.

Upvotes: 2

Mehrdad Afshari

Reputation: 421988

I think return IsValid(); is perfectly valid and readable code.

BTW, I would certainly slap anyone who writes (IsValid() ? true : false) in the face. It's unnecessarily complicated.

PS. This is what svn blame is designed for.

Upvotes: 15

Robin Day

Reputation: 102478

Yes, of course return IsValid(); would be most optimal if the only code you have is as above.

I guess what comes into play though is what else is your function doing? The rest of the code might shed more light on why a developer would put an if statement around IsValid().

After all, if it's just returning IsValid() then why isn't the calling code just checking IsValid() directly rather than having this wrapper method.

Upvotes: 0

Dana

Reputation: 32957

I even sometimes see this some of the legacy code I maintain:

bool retValue;
if (IsValid()) 
{
    retValue = true;
}
else 
{
    retValue = false;
}

return retValue;

Are some programmers paid by the character?

Upvotes: 0

mqp

Reputation: 71935

If you're absent-minded, it's easy to refactor some code from this:

private bool ConsiderTheOstrich()
{
    /* do ostrich things */

    if(someCondition && unpredictableThing == 5)
        return true;
    else
    {
        // log something
        return false;
    }
}

To this:

private void IsValid() { return (someCondition && unpredictableThing == 5); }

/* ... */

private void ConsiderTheOstrich()
{
    /* do ostrich things */

    if(IsValid())
        return true;
    else
        return false; // ostrichlogger logs it for us now
}

Without noticing the extra opportunity for brevity.

Upvotes: 3