Is that the intended behavior?
If you check the JavaDoc of Response<T>
you can read
@Nullable public T body()
The deserialized response body of a successful response.
Javadoc: Response
As it suggests, body()
won't be null
if the response was successful. To check for success you have isSuccessful()
which
Returns true if code() is in the range [200..300).
So the @Nullable
is a valid choice, since the response can be null
in any non-success case, e.g. no network, an invalid request, or some other error.
The hint in your IDE is a lint warning that checks your source code for possible sources of common errors or bugs.
This is why body()
might be null
and why lint reports this as a warning in the first place.
In my point of view response.body() should be immutable, so my check in Picture 1 should not show a warning.
And in theory you're correct. You and I both know that body()
is not mutable, but the problem here is that this lint check has no way of knowing.
T res1 = response.body(); // could be null
T res2 = response.body(); // ...maybe still null?
Lint is a static source and byte code analyzer that helps to prevent common bugs and errors and one of those lint checks tries to prevent NPEs. If you annotate a method @Nullable
all the check knows is that the returned value could be null
and it will raise a warning if you try to operate on the result of the call directly.
// does response return the same value twice? who knows?
response.body() != null && response.body().getCar() != null
The way you handle your response is actually the only way to get rid of the lint warning—other than suppressing or disabling it.
By assigning it to a local variable you are able to ensure that a value is not null
at some point and that it won't become null
in the future, and lint is also able to see that.
CarResponseBody body = response.body(); // assign body() to a local variable
if (body != null && body.getCar() != null) { // variable is not null
CarResponse car = body.getCar(); // safe to access body, car is also not null
// everything is fine!
}
It that the intended behavior?
Yes. @Nullable
is a good way to hint that a method might return null
and you should also use it in your own code if you return null
on some paths because lint can warn of possible NullPointerExceptions.
If a method might return null
you will have to assign it to a local field and check the field for null
values, or you risk that the value could have changed.
Car car = response.getBody(); // car could be null
if(car != null) {
// car will never be null
}
Different options / approaches
I see that you also additionally seem to wrap your response object in an additional layer.
CarResponseBody body = response.body();
Car car = body.getCar()
If you want to remove complexity from your code, you should have a look at how to remove this wrapping *ResponseBody
at an earlier stage. You can do so by registering your own Converter and adding additional handling there. You can see more about this on this Retrofit talk by Jake Wharton
Another completely different approach would be to use RxJava with Retrofit which removes the need to check the response yourself. You'll either get a success or an error, which you can handle the Rx way.