views:

588

answers:

13

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?

+19  A: 

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.

Anton Gogolev
what if it turns to "false" instead of "true". The statement changes as second alternative. Isn't it confusing?
NetSide
@Netside: you still only need to do a `if (!IsSuccessed)` ... very compact imo.
Peter Lillevold
but, I am not convinced about readability. Do you really think it is more readable?
NetSide
Then the design is faulty. You should avoid inherently negative bools: use isOkay instead of isFaulty, hasMembers instead of hasNoMembers, isInWorkMode instead of isInFaultMode, etc. Avoid double and deeper negatives like fire: wrap your head around `if(!noInitErrors)`. The one mistake of designing the system around `if(!isInFaultMode)` as the standard check has bitten me in the ass several times already.
SF.
@SF.: this is convincing. thanks.
NetSide
+6  A: 

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.

Jens
I thought `if(SomeNullableBool)` worked as well.
Svish
Nope, I get a compiler error when trying that. "Cannot implicitly convert type 'bool?' to 'bool' ..."
Jens
+7  A: 

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

TomTom
+1 but I prefer the second.
kenny
+2  A: 

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.

Younes
I think it'd be more readable if you read it like "if is successed" than "if bsuccessed". Seems more natural to me.
Ondrej Slinták
Aha, so a programmer has to write natural language? :)It's smart to use prefixes for a variable. This way you can, without having to search your code, see in one quick look what the type of your variable is. I know that in environments like VS you can just mouseover, but it's usual to use prefixes
Younes
people still do the Hungarian thing? I though we're over that one...
Kobi
Arrghhh! don't start the Hungarian debate again!
Benjol
mehehe, no comments!
Younes
Seriously, `bSuccessed` ? Thought we we're done with Hungarian notation years ago. What "value" do the 'b' give that isn't already there?
Peter Lillevold
Yes, please, stop with the Hungarian thing. If your code says `if(Success)` and you have to wonder if `Successs` is a `bool` or not, I'd say you have other problems...
Svish
The smartness of the hungarian notation must be why it's all but extinct :D
Rune FS
"but it's usual to use prefixes" ... can you reference some statistics on that? :)
Peter Lillevold
+1  A: 

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

hallie
+3  A: 

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.

openshac
I remember doing the same thing when I started programming.
Zaki
+6  A: 

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 firt example may be on par with thi 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.

Rob
Agreed - I always like a "positive" variable i.e. drop the "No" in this case. Makes it much easier to read. Double negatives are not a good thing!
Robert Conn
+1  A: 

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)

Rune FS
+1  A: 

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 ==)

PoweRoy
You shouldn't apply coding conventions from other languages, that are meant to solve problems that don't exist. In C# you get a warning: **Assignment in conditional expression is always constant; did you mean to use == instead of = ?**. C# is awesome.
Kobi
hmm nice didn't know that. I saw coworkers use it in C++, I always use the first one myself.
PoweRoy
+23  A: 

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.

Brian Rasmussen
Good answer. While I agree with the other stylistic based answers, this provides a practical reason for omitting the unnecessary comparison.
Greg Beech
Although this will compile, you do get a warning, even *without Resharper* - "Assignment in conditional expression is always constant; did you mean to use == instead of = ?"
Nick
protect oneself from slippery fingers.
kenny
@Nick: True, and if you turn on warnings as errors VS will stop you from doing this, but with option two you don't have to do anything to avoid the potential bug.
Brian Rasmussen
@Brian: Your answer implies that you need to use tools like Resharper in order to get the warning; Nick is just pointing out that this is not so. Anyway, I agree with the rest of your answer.
Gorpik
@Gorpik: Please see my comment above. VS does warn about this, but R# throws it in your face, which is why I highlighted R# over VS.
Brian Rasmussen
@Nick + @Gorpik I have updated my answer to reflect the info in the comments. Thanks.
Brian Rasmussen
When down voting please leave a comment. Thanks.
Brian Rasmussen
could do this to eliminate the potential of that bug: `if (true == b)` because `if (true = b)` will throw an error in any compiler.
JohnB
@JohnB: But why not just use booleans as booleans. `if (b)` does the same thing, is shorter and without the risk of errors.
Brian Rasmussen
+2  A: 

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.

egarcia
+7  A: 

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?

Konrad Rudolph
+1  A: 

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

Smalltown2000