CODEWITHSUNDEEP
c#coding-stylereadabilityconditional-operator
Chris
Chris

Reputation: 2991

Using true and false as the expressions in a conditional operation

I'm maintaining some code and have found the following pattern a lot:

var isMale = (row["Gender"].ToString() == "M") ? true : false;

instead of this:

var isMale = (row["Gender"].ToString() == "M");

Is there any reason why anyone would do this? Does anyone think the former is more readable or clearer? Is there some sort of old C "gotcha" that this is a hold-over from?

Upvotes: 14

Views: 9578

Answers (9)

SWeko
SWeko

Reputation: 30892

I guess some people are not comfortable with assigning anything other that true or false to a boolean variable. Don't know why this is, but I have observed it quite a few times.

So, from that aspect it looks like:

set sarcasm on

bool isMale = (row["Gender"].ToString() == "M"); //BAAAAD

but

bool isMale = (row["Gender"].ToString() == "M") ? true : false; //BETTER

and

bool isMale;
if (row["Gender"].ToString() == "M") 
    isMale = true;
else 
    isMale = false;   // BEST!

set sarcasm off

Luckily Resharper makes short work of all these anti-patterns.

Upvotes: 4

stinky472
stinky472

Reputation: 6797

Applying the ternary operator ?: for an expression that can only evaluate to true/false (x==y) is just downright redundant (it's just downright redundant [it's redundant]).

The above sentence might be easier to read for someone who doesn't understand English very well as they'll know what to look up first in the dictionary and, if spoken, due to the repetition. Yet for native English speakers, the sentence is awkward and, well, redundant.

In your example, either the operator is being used for documentation purposes, or, without meaning to offend, I suspect that there's a poor understanding of how operators and expressions work.

Whether it's a lack of understanding or some bizarre attempt at documentation, we can do this without redundant operators like this:

var isMale = (row["Gender"].ToString() == "M"); // bool

or...

var isMale = (row["Gender"].ToString() == "M"); // true/false

... or better yet, explicitly specify the appropriate type for 'isMale' instead of relying on implicit typing:

bool isMale = (row["Gender"].ToString() == "M");

I've also seen people pessimize their code this way (or using if/else):

bool something = some_int ? true: false;

There's no need to do this and, while the compiler may optimize this, it is inherently less efficient to rely on branching mechanisms over something as simple as this:

bool something = some_int != 0;

... which has the same effect but without the roundabout process of using conditional branching.

This kind of code is just embarrassing. It's like seeing:

switch (x)
{
    case 1: 
        y = 1;
        break;
    case 2:
        y = 2;
        break;
    case 3:
        y = 3;
        break;
    // etc. for all possible values of x
}

The above would most certainly be considered uber-n00b code by most, yet it is not any less redundant logically than x == y ? true: false or x ? true: false (as opposed to x != 0).

Upvotes: 2

heisenberg
heisenberg

Reputation: 9759

The 

if (somebool) return true;
else return false;

"pattern" gets laughed at pretty much everywhere I've ever worked. No reason to do it in my opinion.

Upvotes: 3

Jerry Coffin
Jerry Coffin

Reputation: 490108

If you can't trust: (row["Gender"].ToString() == "M") to product true or false correctly, then you probably can't really trust: (row["Gender"].ToString() == "M") ? true : false; either. To be sure, you undoubtedly need to repeat it at least a few more times:

(row["Gender"].ToString() == "M") ? true : false ? true : false ? true : false;

Then again, maybe you can't trust a combination of == and ?: by themselves either -- to be really sure, you probably need at least a bit more redundancy:

if ((row["Gender"].ToString() == "M") ? true : false ? true : false ? true : false == true)
    isMale = true == true;
else
    isMale = false != false;

Hmm...maybe I stayed up too late last night... :-)

Upvotes: 10

bobince
bobince

Reputation: 536379

A valid reason? No.

It's usually produced by people who don't really understand that a condition is also in itself an expression, producing a boolean result. In particular, people schooled on a language where this isn't the case, such as many variants of BASIC.

Upvotes: 19

ChrisNel52
ChrisNel52

Reputation: 15143

The 2nd way sure makes the most sense and I think it's easier to read.

The 2nd way is a bit more clever. I wouldn't put it past me to do something like you are finding if I was churning out code on a Friday afternoon and my brain was already gone for the weekend :-)

Upvotes: 1

David
David

Reputation: 218827

Definitely redundant. Could be a leftover practice from another programming language/environment that the original developer was retaining. I could also potentially see a developer viewing the first line as being more readable in that he/she can quickly see that it's setting a boolean when skimming through the code.

Upvotes: 1

John Weldon
John Weldon

Reputation: 40759

I think it's totally redundant.

In my experience, this often happens when a conditional statement evolves over time and ends up with an explicit true or false rather than a sub-statement.

Upvotes: 4

Shaun Bowe
Shaun Bowe

Reputation: 10008

I guess if you get paid by the character that would be valid. Other than that I can't think of any reason.

Upvotes: 16

Related Questions