40

Possible Duplicate:
Is it bad to explicitly compare against boolean constants e.g. if (b == false) in Java?

In this NotePadProvider sample code, I noticed that the author chose the form:

    if (values.containsKey(NoteColumns.CREATED_DATE) == false) {
        values.put(NoteColumns.CREATED_DATE, now);
    }

Over:

    if (!values.containsKey(NoteColumns.CREATED_DATE)) {
        values.put(NoteColumns.CREATED_DATE, now);
    }

Is there any advantage in the first form over the more logical one?

cetinajero
  • 177
  • 1
  • 2
  • 12
ateiob
  • 9,016
  • 10
  • 44
  • 55
  • 6
    It's more readable. – Lukas Knuth Aug 06 '12 at 16:04
  • 14
    @Lukas Knuth Really? I would think the opposite... – ateiob Aug 06 '12 at 16:05
  • Related: [Is it bad to explicitly compare against boolean constants e.g. if (b == false) in Java?](http://stackoverflow.com/questions/2661110/is-it-bad-to-explicitly-compare-against-boolean-constants-e-g-if-b-false-i) – BalusC Aug 06 '12 at 16:06
  • 4
    In the first example you see at first glance that the expected outcome should be false. In the latter example you can only guess what is expected. Also it is easy to miss the exclamation mark. – Torsten Walter Aug 06 '12 at 16:07
  • 2
    `if (b==false)` is more verbose and harder to read. If you want to make it even more verbose, you could use `if (b == false == true == true)` (borrowed from http://stackoverflow.com/questions/2661110) – Steve Kuo Aug 06 '12 at 16:10
  • 1
    how about `if(b != true)` to complete things. Coming from a C-Background I prefer the verbose version for clarity, however (assuming the compiler doesn't optimize that away) the verbose appoach creates more processing: `Read valaue one` `read value two` `compare` `proceed according to output` as opposed to `read value` `proceed according to value` – Mark Jul 15 '14 at 12:20

7 Answers7

63

Apart from "readability", no. They're functionally equivalent.

("Readability" is in quotes because I hate == false and find ! much more readable. But others don't.)

Ry-
  • 218,210
  • 55
  • 464
  • 476
  • 1
    I think it even goes beyond the readability argument. ! (the bang operator) was created for this purpose. You should use it because it helps you write these statements in a succinct manner. You CAN write code using a more limited range of operators but your code will suck from a readability standpoint and won't be following conventions everyone uses. – user2481095 Jan 16 '17 at 22:02
8

Mostly READABILITY. When reading others code, it is much more intuitive to read as NOT CONTAINS KEY !values.containsKey(NoteColumns.CREATED_DATE) instead of reading CONTAINS KEY IS FALSE (values.containsKey(NoteColumns.CREATED_DATE) == false).

devang
  • 5,376
  • 7
  • 34
  • 49
6

This is a style choice. It does not impact the performance of the code in the least, it just makes it more verbose for the reader.

Woot4Moo
  • 23,987
  • 16
  • 94
  • 151
5

- Here its more about the coding style than being the functionality....

- The 1st option is very clear, but then the 2nd one is quite elegant... no offense, its just my view..

Kumar Vivek Mitra
  • 33,294
  • 6
  • 48
  • 75
4

No. I don't see any advantage. Second one is more straitforward.

btw: Second style is found in every corners of JDK source.

卢声远 Shengyuan Lu
  • 31,208
  • 22
  • 85
  • 130
3

Note: With ConcurrentMap you can use the more efficient

values.putIfAbsent(NoteColumns.CREATED_DATE, now);

I prefer the less verbose solution and avoid methods like IsTrue or IsFalse or their like.

Peter Lawrey
  • 525,659
  • 79
  • 751
  • 1,130
  • Excellent. I love this. Since [ConcurrentMap](http://developer.android.com/reference/java/util/concurrent/ConcurrentMap.html) isn't derived from [ContentValues](http://developer.android.com/reference/android/content/ContentValues.html) and there isn't a direct connection between the two, do you have any suggestions to convert the passed `ContentValues` parameter to a `ConcurrentMap`? Oh wait, `ConcurrentMap` is an **interface**! Let me check this, I never came across `ConcurrentMap` before. – ateiob Aug 06 '12 at 18:13
  • Interesting, it doesn't even extend Map and I am not sure its thread safe. You could write a `putIfAbsent(ContentValues, NoteColumns.CREATED_DATE, now)` method. ;) – Peter Lawrey Aug 06 '12 at 18:16
1

The first form, when used with an API that returns Boolean and compared against Boolean.FALSE, will never throw a NullPointerException.

The second form, when used with the java.util.Map interface, also, will never throw a NullPointerException because it returns a boolean and not a Boolean.

If you aren't concerned about consistent coding idioms, then you can pick the one you like, and in this concrete case it really doesn't matter. If you do care about consistent coding, then consider what you want to do when you check a Boolean that may be NULL.

Sam
  • 2,939
  • 19
  • 17
  • 1
    This could be a truly enlightening answer, if I only understood where you see a `Boolean`. All I see there is a `boolean`. Please explain. – ateiob Aug 06 '12 at 16:18
  • "In this concrete case it really doesn't matter." As you rightly point out, there is no Boolean shown above. My concern was for future code which might return to you a Boolean type and might be null. If you believe this will, "never happen," then this answer isn't very useful. – Sam Aug 06 '12 at 18:59