1

My IDE is marking the following code snippet's second function call as a potential source for a NullPointerException, and I'm unsure if that is actually the case.
The snippet:

Sprite tmp = coordinateUtils.getSpriteUnderEntity(p);
if( isOutOfBounds(p)
    ||(p.collides(tmp)&&Resource.isImpenetrable(tmp.getType()))
){
//do stuff
}

(For clarification coordinateUtils.getSpriteUnderEntity() takes a Sprite as argument for its coordinates then returns the (background) Sprite currently under it, or null if the Sprite has nothing under it)

p is a projectile which is a kind of Sprite, which has a collides method while tmp is a Sprite. Here's a simplified version of that method (simplified as in the actual collision logic will be hidden as it's irrelevant for this question).

public boolean collides(Sprite other){
    return other != null && doesItCollide(other);
}

other (tmp in the snippet) is in fact nullable but since collides is called before getType(), and that would return false if other (tmp) was null I can't ever run into a NullPointerException here, right?
To rephrase the question, java's evaluation is lazy here, right?

devor110
  • 121
  • 8
  • What IDE are you using? Can you show what the code before `p.collides(tmp)&&Resource.isImpenetrable(tmp.getType()` is, so that it is a [mcve]? You can probably add some sort of annotations to `collides` so that the IDE can figure out that `collides` must return false if `other == null`. – Sweeper Mar 14 '22 at 15:03
  • The compiler only sees that `tmp` is nullable at the time it encounters `p.collides(tmp)`. It doesn't "know" per se that the method returns `false` if `tmp` is really `null`. And since nothing in that same context (the whole condition) is ensuring that `tmp` will definitely not be null afterwards, the IDE will give you the warning. – QBrute Mar 14 '22 at 15:04
  • Also note: if `tmp` would be a field of the surrounding class, theoretically, it could be not null for the first call, but change to null before the `&&` condition gets evaluated. – GhostCat Mar 14 '22 at 15:06
  • @GhostCat he is not accessing tmp.... into a nested field, so what you say can not be the case. The reference can not change the tmp even if it is a property of a class – Panagiotis Bougioukos Mar 14 '22 at 15:33
  • @PanagiotisBougioukos If `tmp` is a field of that class, and the same object of that class is processed by two different threads, and the corresponding code doesn't account for that, there can be a race condition. `&&` doesn't prevent another thread from running and making updates to fields. – GhostCat Mar 14 '22 at 15:37
  • @GhostCat no because tmp would be only a reference to some object. Even if some other thread changed another copied reference in another function, the original object reference would still point to the starting object. Java is passByValue in functions which means a copy of the reference is passed into the function parameter – Panagiotis Bougioukos Mar 14 '22 at 15:40
  • @GhostCat what you mention could be the case if he nullified tmp.field1 and then yes field1 could be null later down the road. But considering he uses only a specific reference tmp and not a inner field, this can't be the case. It will always pass a copy of the reference tmp in any method – Panagiotis Bougioukos Mar 14 '22 at 15:45
  • @PanagiotisBougioukos Within`p.collides(tmp)&&Resource.isImpenetrable(tmp.getType()`, when `tmp` is a field of the class, call by value doesnt matter. The first method might see a non-null tmp, the other might not. tmp is checked, and then a method on tmp is called. – GhostCat Mar 14 '22 at 15:45
  • @Sweeper I'm using intelliJ Idea (community), which in my experience handled similar issues very well in the past. However it has been broken for a few days (such as marking classes clearly in scope as inaccessible as thus showing a compile-time error, even though my code compiles and runs just fine) so maybe this issue could also originate from that – devor110 Mar 14 '22 at 16:40

1 Answers1

2
public boolean collides(Sprite other){
    return other != null && doesItCollide(other);
}

No it won't be evaluated. Java && operator “short-circuits”, that means it evaluates the left expression first and only if it is true, evaluates the right one. It is possible that the IDE is not “smart enough” to figure such cases.

Different thing is that the method can be marked as throwing the NPE. In this case, you must (or you should) handle it, because it does not have to be caused by the argument, but by something inside the method.

See also this answer.

jiwopene
  • 3,077
  • 17
  • 30