Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Using nullable Booleans #18

Open
cypressious opened this issue Jun 1, 2016 · 7 comments

Comments

@cypressious
Copy link

commented Jun 1, 2016

When you have a variable val value: Boolean? you have a couple of possible ways to test it for true:

  1. if (value != null && value) (doesn't work with mutable properties)
  2. if (value ?: false)
  3. if (value == true)

I prefer version 3 because it makes the intent very clear.

In many cases you have an additional possibility: Write an extension function that works on nullable types. Example: nullableString.isNullOrEmpty()

@matklad

This comment has been minimized.

Copy link

commented Jun 10, 2016

I strongly prefer 2 because it is clear that you are dealing with a nullable boolean.

In contrast, 3 can be easily confused with a typical beginners mistake of if (condition == true).

@yole

This comment has been minimized.

Copy link
Owner

commented Jun 12, 2016

@matklad I think that the inversion of the boolean that you have to do in option 2 (the statement is executed when the value is true, whereas the value that you have in the code is false) is significantly worse than the (quite harmless) uncertainly of the nullability of the value that might occur with option 3.

@voddan

This comment has been minimized.

Copy link

commented Jun 12, 2016

Can we have a bright highlighting in IDE for value == true in if (value == true), so that no one attempts to "refactor it"?

@yole

This comment has been minimized.

Copy link
Owner

commented Jun 12, 2016

I don't think that's actually necessary, because the refactored code will not compile. We do need to highlight if (value == true) when value is a regular Boolean, with a quickfix to simplify.

@voddan

This comment has been minimized.

Copy link

commented Jun 12, 2016

I think it is somewhat a frustrating experience to attempt to refactor if(value == true) only to find out that no mistake was made. Without a visual clue those two situations are indistinguishable. The IDE code-smell warning may or may not be enough.

@matklad

This comment has been minimized.

Copy link

commented Jun 12, 2016

@yole hm, agree. Then it is the question of educating people about the fact that == true is an idiom and not a code smell.

And just a terrible idea, can a special syntax be invented for nullable booleans, like if (value?) perhaps?

@yole

This comment has been minimized.

Copy link
Owner

commented Jun 12, 2016

We've discussed the possibility of handling nullable booleans in if in a special way, and decided that the current syntax is the most explicit about the actual handling of null (which can be different in different cases).

@voddan voddan referenced this issue Jun 16, 2016

msfjarvis added a commit to msfjarvis/viscerion that referenced this issue Oct 25, 2018

Change code style around nullable booleans
Referencing from yole/kotlin-style-guide#18, I prefer this variant.

Signed-off-by: Harsh Shandilya <msfjarvis@gmail.com>

msfjarvis added a commit to msfjarvis/viscerion that referenced this issue Oct 25, 2018

treewide: Get rid of all double bangs (#64)
* treewide: Get rid of all double bangs

Replace with proper nullity handling

* Fix databinding errors

* Change code style around nullable booleans

Referencing from yole/kotlin-style-guide#18, I prefer this variant.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.