views:

818

answers:

11

Is it bad to write:

if (b == false) //...

while (b != true) //...

Is it always better to instead write:

if (!b) //...

while (!b) //...

Presumably there is no difference in performance (or is there?), but how do you weigh the explicitness, the conciseness, the clarity, the readability, etc between the two?

Update

To limit the subjectivity, I'd also appreciate any quotes from authoritative coding style guidelines over which is always preferable or which to use when.


Note: the variable name b is just used as an example, ala foo and bar.

+6  A: 

I've never seen the former except in code written by beginners; it's always the latter, and I don't think anyone is really confused by it. On the other hand, I think

int x;
...
if(x) //...

vs

if(x != 0) //...

is much more debatable, and in that case I do prefer the second

Michael Mrozek
@Michael, Java doesn't implicitly convert int to boolean like in C++, but otherwise agree 100%; the former is sloppy newbie syntax, and the latter is preferred.
Michael Aaron Safyan
Oh, I didn't even realize we were talking about Java :)
Michael Mrozek
@Michael: it was in the tags, but now also mentioned in title. Sorry for confusion.
polygenelubricants
+1  A: 

In my opinion it is simply annoying. Not something I would cause a ruckus over though.

ChaosPandion
+24  A: 

It's not necessarily bad, it's just superfluous. Also, the actual variable name weights a lot. I would prefer for example if (userIsAllowedToLogin) above if (b) or even worse if (flag).

As to the performance concern, the compiler optimizes it away at any way.

Update: as to the authoritative sources, I can't find something explicitly in the Sun Coding Conventions, but at least Checkstyle has a SimplifyBooleanExpression module which would warn about that.

BalusC
+1 for recommending long, descriptive variable names. However, I think "if (userIsAllowedToLogin())" is preferable to "if (userIsAllowedToLogin()==true)" as the former is more readable.
Michael Aaron Safyan
Truely superfluous code *is* necessarily bad.
Michael Borgwardt
@Michael, did you forget a `!` ?
Thorbjørn Ravn Andersen
I'd prefer `if( canAuthenticate() )` or `if( isAuthorized() )`. I find `userIsAllowedToLogin` ambiguous. Does it mean that the user can view the login page (e.g., the IP address is not banned) or does it mean that the user has been authenticated (e.g., correct name and password)?
Dave Jarvis
@Dave: It was just a bit exaggerated example. As long as the variable name is self-explaining enough.
BalusC
@BalusC: I know; it's a pet peeve. Long method names -- like `isCurrentUserLoggedInAsRole` -- are sometimes a symptom of a larger problem: a misplaced method. (For example, the `User` class should know its assigned roles: `user.isAssigned( role )`.)
Dave Jarvis
+2  A: 

I prefer the first, because it's clearer. The machine can read either equally well, but I try to write code for other people to read, not just the machine.

Head Geek
+1 Definitely write code that's easier for people to read.
R0MANARMY
+20  A: 

IMO, yes you should should not use the first style. I have seen people use:

if ( b == true ) 
or 
if ( b == false )

I personally find it hard to read but it is passable. However, a big problem I have with that style is that it leads to the incredibly counter-intuitive example you showed:

if ( b != true )
or
if (b != false )

That takes more effort on the part of the reader to determine the authors intent. Personally, I find including an explicit comparison to true or false to be redundant and thus harder to read, but that's me.

Thomas
+1 This is another best answer in this topic. They are indeed (a tad) harder to interpret *quickly.* At least, for me.
BalusC
+3  A: 

Usually I prefer the ! form, but:

  1. if (obscureFcn("file") == false)

  2. if (!obscureFcn("file"))

The first way tells you that obscureFcn() returns bool, you don't even have to think about it. The second could be int, char, some kind of pointer, etc. One night at 5am when you're debugging a production crash you'll be so glad you took the more verbose option!

(I realize that the recommended form of [2] is if (obscureFcn() == 0), but that's one I find verbose.)

egrunin
This is Java, not C. You cannot use `obscureFcn() == 0` instead of `!obscureFcn()` or *vice versa*.
Stephen C
Ah, that wasn't in the text...never mind then.
egrunin
+10  A: 

The overriding reason why you shouldn't use the first style is because both of these are valid:

if (b = false) //...

while (b = true) //...

That is, if you accidentally leave out one character, you create an assignment instead of a comparison. An assignment expression evaluates to the value that was assigned, so the first statement above assigns the value false to b and evaluates to false. The second assigns true to b, so it always evaluates to true, no matter what you do with b inside the loop.

Alan Moore
I wouldn't say "overriding" reason. Unless you time travel to 15 years ago your compiler will warn about this typo.
John Kugelman
`while (b = false)` is valid Java syntax (if `b` is a `boolean`) and a compiler will not catch it.
mobrule
John probably meant your IDE. Note this is not valid in C#... jab!
Stephen Swensen
Yes, IDE's and style checkers will warn you about this kind of thing. I wouldn't be surprised if the command-line compiler can detect it as well, although it doesn't warn you by default. My point is that this isn't *just* a style choice.
Alan Moore
@John and @mobrule - Eclipse warns about this, but only if you set the "Parameter assignment" option true (it defaults to false).
CPerkins
+4  A: 

IMHO, I think if you just make the bool variable names prepended with "Is", it will be self evident and more meaningful and then, you can remove the explicit comparison with true or false

Example:

isEdited  // use IsEdited in case of property names
isAuthorized // use IsAuthorized in case of property names

etc

Mahesh Velaga
The convention is `isEdited`, not `IsEdited` (the latter would be a class)
Thorbjørn Ravn Andersen
Yikes! sorry I meant the same, thanks for pointing out
Mahesh Velaga
+5  A: 

This is strongly a matter of taste.

Personally I've found that (!a) is a lot less readable than (a == false) and hence more error prone when maintaining the code later, and I've converted to use the latter form.

Basically I dislike the choice of symbols for logic operations instead of words (C versus Pascal), because to me a = 10 and not b = 20 reads easier than a == 10 && !(b==20), but that is the way it is in Java.

Anybody who puts the "== false" approach down in favour of "!" clearly never had stared at code for too long and missed that exclamation mark. Yes you can get code-blind.

Thorbjørn Ravn Andersen
+1  A: 

Personally, I would refactor the code so I am not using a negative test. for example.

if (b == false) {
   // false
} else {
   // true
}

or

boolean b = false;
while(b == false) {
  if (condition)
      b = true;
}

IMHO, In 90% of cases, code can be refactored so the negative test is not required.

Peter Lawrey
but don't take it to far. believe it or not, I have already seen code like this:if (a) ;else doSomething();
Axel
In which case I would prefer if (!a) doSomething(); But it likely you will find that you can change the code so that 'b' stores true instead of false and false instead of true and there is no need for a negation. IMHO it is fairly rare you need if (a) in one place *and* if (!a) in an unrelated block of code.
Peter Lawrey
+1  A: 

I would say it is bad.

while (!b) {
    // do something 
}

reads much better than

while (b != true) {
    // do something 
}
fastcodejava