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.

CodePudding user response：

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¹.

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.

---

^{1 - I assume that is what you mean by "dip that method". I don't recognize that English usage though. You should avoid using slang terms that most StackOverflow readers won't understand.}

CodePudding user response：

You should never be concerned about a yellow IDE warning, but IF it does happen to matter, I would use option 3. Option 1 will have an unnecessary assert, option 2 will throw your plans as a coder away.

I would like to say it like this: If the IDE doesn't understand your code, add the if statement. It not only makes your code a lot easier to read, but also makes it as a precaution, where if you ever change your code it will be unaffected.

Happy coding! :)

CodePudding user response：

I agree with the other answers here, but just wanted to point out that, to me, the logic of that block is completely backwards. So instead, what I'd do would be to rewrite the whole block so that it flows more naturally.

private static boolean sendLore( Player player, ItemStack mainHandItem )
{   if  ( mainHandItem.getItemMeta().hasLore() )
    {   player.sendMessage( ChatColor.GREEN   "The lore is: " );
        for ( Component loreItem : mainHandItem.getItemMeta().lore() )
        {   var Message = Component
                          .text( ChatColor.GRAY   String.valueOf( i   1 )   ") "   ChatColor.RESET )
                          .append( lineMessage( loreItem ) );
            player.sendMessage( Message );
        }
        return true;
    }
    else
    {   player.sendMessage( ChatColor.RED   "This item doesn't have a lore." );
        return false;
    }
}

Presumably this would also make the warning go away.