7

About this function: org.apache.commons.lang3.BooleanUtils.isFalse(Boolean bool) and the similar isTrue, my co-work (less experienced) use it for every boolean in the code. I am trying to convince him that it is not a good practice because make the code confusing. In my POV, it should be used ONLY if the boolean to be tested is a type of non-primitive Boolean and it can be null. Actually, even this I think it is unnecessary, because the implementation of this function is simply Boolean.TRUE.equals(bool) or Boolean.FALSE.equals(bool). Anyway, I think it is totally crazy do something like:

boolean isReady = true;

if (BooleanUtils.isTrue(isReady)) {
   // ...
}

when you cold simply do

if (isReady) {
   // ...
}

or (!isReady) for the opposite.

His only argument to use this is "it is easy to read". I just can't accept this argument. Am I wrong? What arguments can I use to convince him that no useless code is better than useless code in this case?

Thank you guys.

Felipe
  • 329
  • 1
  • 4
  • 13
  • 1
    `BooleanUtils.isTrue(isReady)` is really hard to read for me because it is tooooooo looooong. – Bryan Chen Jun 12 '13 at 05:08
  • 1
    For the record: I agree with you completely. These methods are totally useless, since they are exactly as long as the implementation and no more clear in meaning. – Michael Borgwardt Jun 12 '13 at 08:46
  • 2
    Never. Also, in my experience, apache apis are very often badly designed. – DPM Jun 12 '13 at 09:18
  • 1
    this is only useful when you use `Boolean` instead of `boolean`, cause then there are 3 different possible values: true, false and null. When null, a NullPointerException will be thrown. See http://stackoverflow.com/questions/9479806/boolean-object-and-boolean-variable-issue-in-java – Salvatorelab Jul 24 '14 at 15:40
  • @Salvatorelab: you forgot FILE_NOT_FOUND – kevin cline May 24 '18 at 10:37

3 Answers3

10

It's absurd to use BooleanUtils.isTrue(isReady) when isReady is a boolean, not a Boolean. Your colleague seems like one of those guys who thinks that the more code he writes, the more productive he is. Maybe he should go find a job programming in Ada.

kevin cline
  • 33,608
  • 3
  • 71
  • 142
  • Or maybe they pay him per line. – Martin Wickman Jun 12 '13 at 09:07
  • 3
    I rather suspect that the colleage is simply a pretentious noob. He probably read or heard once, somewhere, that using "BooleanUtils.isTrue" makes a code better. He either did not understand the explanation behind it, or he forgot it soon afterwards. But he is happy to remember *a rule*, and now he applies it dogmatically. Give him enough time, and he will learn hundreds of other rules. (Never use "==", always use "equals". Never make subclasses, always encapsulate. Every class must have an interface. Always use design patterns, the more the better. Always/Never use synchronized methods. Etc.) – Viliam Búr Jun 12 '13 at 15:13
  • sometimes this slavish rule adherence turns into mass hysteria, e.g. the Java style rule that constants should be all upper case, carried over from C, where it was a work-around for the use of `#define` to define constants. – kevin cline Jun 12 '13 at 19:28
2

1) Bugs come from code. The more code you have, the more bugs. Always try to write less code, even if that's just a few words here and there.

2) Easy isn't Easiest. Which is easiest to read?

Try compromising with if(isReady == True), if he really wants that explicit comparison.

Narfanator
  • 222
  • 2
  • 6
  • 8
    `if (isReady == True)` ?? When does it end? Why not `if ((isReady == True) == True)` ? – kevin cline Jun 12 '13 at 04:43
  • 1
    @kevincline: I'd use `if(isReady == True)` in a language that implicitly casts non-boolean values to booleans in if conditions, as it improves the type-safety of the underlying language: `if(0 == true)` ought to result in a compilation warning. – Aidan Cully Jun 12 '13 at 13:59
  • @kevincline I'm trying to figure out /why/ the op's antagonist wants that syntax, and the best I can come up with is he wants to see that you're checking against True. – Narfanator Jun 12 '13 at 19:42
1

I think the goal should be to have descriptive boolean variable names, as you have shown in your example.

Dan Teesdale
  • 119
  • 3
  • 1
    I thought exactly the same thing today rethinking about this. Using descriptive boolean kills his argument of "it is easy to read". The above answer also makes a good point: if he find "isReady == true" ugly, so there is no reason to think BooleanUtils.isTrue(isReady) beautiful. Tomorrow I'll try theses arguments. – Felipe Jun 12 '13 at 04:39
  • 1
    @FelipeMicaroniLalli good luck! – Dan Teesdale Jun 12 '13 at 04:46