Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

@Nonnull with different IDEs - warn about unnecessary null checks

I'm working in an environment where developers use different IDEs - Eclipse, Netbeans and IntelliJ. I'm using the @Nonnull annotation (javax.annotation.Nonnull) to indicate that a method will never return null:

@Nonnull
public List<Bar> getBars() {
  return bars;  // this.bars is a final, initialized list
}

I'd like other developers to get a warning if they do one of the following:

  1. Change the method to return null without removing the @Nonnull annotation
  2. Unnecessarily check for null for methods that are explicitly defined with @Nonnull: if (foo.getBars() == null) { ... do something ... }

The first scenario is supported e.g. by IntelliJ. The second is not; the clients are not warned that checking for null is unnecessary.

We're in any case planning to move towards returning clones of collections instead of the collections themselves, so is it better to forget @Nonnull and do this instead:

public List<Bar> getBars() {
  return new ArrayList<Bar>(bars);
}

Edit:

To clarify, I'm not considering changing IDE's for this. I'd like to know whether what I described above is supported by the mentioned IDEs - or alternatively, if there is a good reason as to why it is not supported.

I get the point about not relying too much on contracts. However, if I write getBars() with the style in the last paragraph (return a clone of the list), then e.g. IntelliJ flags a warning for any code that does

if (foo.getBars() == null) { ... }

If you choose to follow this warning and remove the null check, you seem to be equally reliant on the getBars() implementation not changing. However, in this case you seem to be depending on implementation details instead of an explicit contract (as is the case with @Nonnull).

Edit #2:

I'm not concerned about execution speed, null checks are indeed very fast. I'm concerned about code readability. Consider the following:

Option A:

if ((foo.getBars() == null || foo.getBars().size() < MAXIMUM_BARS) && 
    (baz.getFoos() == null || baz.getFoos().size() < MAXIMUM_FOOS)) { 
  // do something
}

Option B:

if (foo.getBars().size() < MAXIMUM_BARS && 
    baz.getFoos().size() < MAXIMUM_FOOS) {
  // do something
}

I think Option B is more readable than Option A. Since code is read more often than it is written, I'd like to ensure all code I (and others in our team) write is as readable as possible.

like image 614
Lauri Harpf Avatar asked Jan 27 '14 08:01

Lauri Harpf


People also ask

What does @NonNull annotation do?

@NotNull The @NotNull annotation is, actually, an explicit contract declaring that: A method should not return null. Variables (fields, local variables, and parameters) cannot hold a null value.

How do I avoid multiple nulls in Java 8?

We can get rid of all those null checks by utilizing the Java 8 Optional type. The method map accepts a lambda expression of type Function and automatically wraps each function result into an Optional . That enables us to pipe multiple map operations in a row.

Why is checking for null a good practice?

You will often want to say "if this is null, do this", and continue executing. Without the null check you won't get that chance, your whole call stack will be unwound and Unity will go on with the next Update() or whatever method it's going to call. Another is that exception handling is not free.

How do you avoid null?

One way of avoiding returning null is using the Null Object pattern. Basically you return a special case object that implements the expected interface. Instead of returning null you can implement some kind of default behavior for the object. Returning a null object can be considered as returning a neutral value.


1 Answers

I would recommend the following:

  • Stick with your @Nonnull and @Nullable annotations just like you are doing it already.
  • Define and enforce a default. It doesn't really matter what the default is, but it should be the same across your entire code base.
  • Use FindBugs to check for your cases 1 and 2. FindBugs has plugins for most IDEs, I saw Eclipse, Netbeans, and IntelliJ mentioned, but there are more. (The number 2 case is covered by the RCN_REDUNDANT_NULLCHECK_OF_NONNULL_VALUE rule. I tested it. Just thought I should mention that after reading DavidHarkness' comment elsewhere on this page.)

This approach is IDE agnostic and works also in an external build environment like Hudson or Jenkins.

like image 192
barfuin Avatar answered Sep 28 '22 07:09

barfuin