Search code examples
javarefactoringassert

Should an assert be used to fix an IDE warning?

To be more concise, which one of these methods should I use?

For context, the warning (marked in yellow in the image below) says that the lore variable might be null, although I handle it (marked in blue).

These are the solutions I came up with:

  1. The "assert so that he shuts up":

  2. The "remove that method and use another":

  3. The "unnecessary if":

I used the second option, just assert it, but I know that I shouldn't use assertion for such unimportant stuff.

Solution

  • Should an assert be used to fix an IDE warning?

    No it shouldn't. The reason is that the assert is not guaranteed to prevent the NPE ... should the value be null at runtime. Why? Because the execution of assert statements is conditional on the JVM being run with an appropriate -ea option. The default is that assertion checking is off.

    And besides, I suspect that an assert wouldn't suppress the warning anyway, at least for some IDEs and bug checkers.

    As @Aarav writes adding a possibly redundant if test for null is the best solution. Using a temporary variable is a good idea too, especially if it avoids a costly extra method call.

    There is no need to worry that the redundant test will have a runtime overhead. The JIT compiler is good at optimizing out redundant null checks. For example:

    if (obj != null) {
    obj.someMethod();
}

    In addition to the explicit null test, there is a 2nd implicit test for null when you call someMethod on obj. But the JIT compiler will remove the 2nd test because it "knows" that it has already checked that obj is not null.