Reputation: 3890

Confusing If Statement?

I always use If statement (In C#) as (1. Alternative);

if (IsSuccessed == true)
{
   //
}

I know that there is no need to write "== true" as (2. Alternative));

if (IsSuccessed)
{
   //
}

But, I use it because it is more readable and cause no performance issue. Of course, this is my choice and I know many software developers prefer first alternative. What is the best usage, and Why?

Upvotes: 12

Answers (13)

Brian Rasmussen

Reputation: 116401

I don't like the first option. Not only is it redundant, but a simple typo will introduce a bug.

Consider this

bool b = false;

if (b = true) {
   Console.WriteLine("true");
}

Obviously the code will output "true" but that was probably not the intention of the programmer.

Fortunately tools like Resharper warns against this, but it compiles with the default settings (*).

Using the bool directly will remove the issue entirely.

(*) To be fair, VS also warns against this and if you turn on Warnings as errors it won't even compile.

Upvotes: 25

Rob

Reputation: 45771

If the name of the boolean value makes it perfectly clear what it is, then I'd always opt for version 2. However, sometimes you're stuck with a particularly obtuse variable name that you can't change, at least, can't change right now... Refactoring is all well and good, but I try and avoid refactoring too heavily when making functional changes to the code as well.

For example:

if (!NoDropDownInHeader == true)
{
  // Activates when there *is* a dropdown in the header)
}

I've actually seen this particular example in production code and simplified it down to:

if (NoDropDownInHeader == false)
{
 // Activates when there *is* a dropdown in the header
}

And I personally think that both examples are more readable (although arguably the first example may be on par with this one for difficulty of mental parsing) than:

if (!NoDropDownInHeader)
{
 // Activates when there *is* a dropdown in the header
}

Note: Yes, I know the variable is badly named, but changing it in the multitude of places that it was present was outside the scope of the change I was making due to the number of places if would affect.

Upvotes: 4

Rune FS

Reputation: 21742

I'll go for the second. It's easier for me at least. In the first alternative I always wonder why the comparison is made. Check the type of the left hand side just to be sure that no developer on acid overloaded the == operator making comparison between his class and bool an option.
The first also leads to bugs the second won't.
if(a) might need to be change to if(a||b) or if(a&&b) in the first version it might end up as if(a == true || b) and if(a == true && b) in the former b is redundant and the latter equals if(a==b)

Upvotes: 1

smalltown2k

Reputation: 810

More typing means more chances for bugs. Option 2 all the way...

Upvotes: 2

RvdK

Reputation: 19790

What i see most is: (what I do)

if (IsSuccessed)
{
   //
}

and as alternative for in C++, for C# it's not needed (see comment):

if (true == IsSuccessed)
{
   //
}

The alternative is to prevent yourself for making the mistake to assign instead of compare. (= vs ==)

Upvotes: 1

Konrad Rudolph

Reputation: 545568

I claim that someone favouring the first alternative has a sketchy grasp of boolean logic. They might “understand” it intellectually, but they certainly don’t grok it; they haven’t internalized this way of thinking.

After all, does anyone every use the following idiom? “If it’s raining tomorrow is false we may go swimming” – NO, of course not. nobody says something like this, it’s ridiculous. What argument supports the claim that this idiom suddenly becomes clear when applied in a programming (as opposed to natural) language?

Upvotes: 7

kikito

Reputation: 52641

The two expressions are equivalent in C#, but be aware than in other languages they are not.

For example, in C++, the first option accepts only a boolean value with a value of true. Any other value on IsSuccessed will invalidate the condition.

The second option accepts any value that is "truthy": values like 1, or any non-zero, are also considered valid for the if.

So these conditions will validate:

// Truthy validation (second option)
if(1) {...} //validates
if(2) {...} //validates

While these others will not:

// Equals to true validation (first option)
if(1==true) {...} // does not validate
if(2==true) {...} // does not validate

Again, this doesn't apply to C#, since it only accepts booleans on ifs. But keep in mind that other languages accept more than just booleans there.

Upvotes: 2

openshac

Reputation: 5165

I used to write "== true" because I thought it was clearer and more explicit, but decided to change. Now it always seems much clearer without, you'll just get used to it.

Upvotes: 3

hallie

Reputation: 2845

i use the first when i started programming but kinda get used to the second option. It also saves time type extra letters.

Upvotes: 1

Younes

Reputation: 4823

I would settle for your second option aswell. There is in my opinion no need to write the

if (IsSuccessed == true) 
{ 
   // 
}

In fact, I totally dislike the using of == true for a boolean, since it has no extra value. AND: You have to type less characters, which obviously is an advantage :p. To be honest i would also rewrite the boolean to bSuccessed, since it's a boolean.

Upvotes: 2

Jens

Reputation: 25563

I prefer the second alternative. I think it's more readable, but the first alternative has the advantage of staying the same if you need to use Boolean? for some reason.

Upvotes: 6

TomTom

Reputation: 62093

Totally style dependant. Seriously. Go with whatever you like for your own stuff, whatever is the enfoced style at your work.

Upvotes: 8

Anton Gogolev

Reputation: 115731

I'd personally go for the second option. It reads more naturally and shows that a programmer is actually aware of a built-in bool type, which is a first-class citizen.

Upvotes: 19

Confusing If Statement?

Answers (13)

Related Questions