I have a class where the compiler tells that I HAVE to annotate it with @NonNullByDefault
[WARNING] org.openhab.binding.nanoleaf.internal.model.Ct.java:[20]
Classes/Interfaces should be annotated with @NonNullByDefault
So I do
@NonNullByDefault
public class Ct {
private int value;
private Integer max;
as it is a model that MUST have null values because it is used to generate JSON where the values MUST HAVE null values in certain situations I annotate the fields with @Nullable
@NonNullByDefault
public class Ct {
private int value;
private @Nullable Integer max;
private @Nullable Integer min;
which produces an even worse ERROR by the compiler
[ERROR] /Users/stefanhohn/Development/checkout/openhab2-addons-branch/bundles/org.openhab.binding.nanoleaf/src/main/java/org/openhab/binding/nanoleaf/internal/model/Ct.java:[41,983] Null type mismatch (type annotations): required '@NonNull Integer' but this expression has type '@Nullable Integer'
[ERROR] /Users/stefanhohn/Development/checkout/openhab2-addons-branch/bundles/org.openhab.binding.nanoleaf/src/main/java/org/openhab/binding/nanoleaf/internal/model/Ct.java:[49,1109] Null type mismatch (type annotations): required '@NonNull Integer' but this expression has type '@Nullable Integer'
I am bit cautious of saying that it might have helped (the weird thing is that in those cases when I try out different things on other places other warnings quickly pop up or come back again âŠsighâŠ).
I marked the method as
public @Nullable Integer getMin() {
return min;
}
However, this leads to the next problem when using the method:
Is there a reason not to make the fields primitive int in the first place? Because from your code snippets it doesnât look like null is handled differently as 0. This doesnât give an answer to the handling of nulls in general of course.
The null handling can certainly be difficult and the warnings are for worse case, meaning in practice it will never be a problem. Iâm also not completely convinced the null annotations are the most elegant way to help with this. The underlying assumption for the warnings is, everything can be run a threaded environment. So in this specific case between the first call to getMin and the second call andother thread could have set the value to null, so the second call would be null. For variables that are really run threaded this makes sense (for example the scheduled task reference), but for most other objects it can be more of a burden to work around it than it really could cause a problem. To cover this first assign to a local variable and than use that variable. In general I prefer to use primitives as much as possible.
So to wrap this up the most elegant way then is probably to use this implementation:
@Nullable Integer max = colorTemperature.getMax();
int colorMax = (min == null) ? 0 : min;
As of
Is there a reason not to make the fields primitive int in the first place?
Yes, there is. There are situations where the object is and needs to be completely âemptyâ, i.e. the values of the max and min are not set. The CT object will then be serialized to JSON and due to the API requirements on the nanoleaf must not be set and kept empty in the JSON.
Iâm also not completely convinced the null annotations are the most elegant way to help with this.
cheers
Stefan
Thanks, that eases my pain a bit though still I like to provide as much clean code as possible before we go live, so I really strive to get rid of all warnings (otherwise by the way the broken window effect kicks in eventually).
If itâs a case that these fields should never be serialized when send to Nanoleaf, but do need to be read when read from Nanoleaf you can use the annotation @Expose.
You can also either let the get methods simply do the null check and return a primitive or add additional methods for returning primitive values. That way you donât need to pollute your other code with all these local variables.
Ok, I will probably keep that in mind for the next release and put that on my list, @hilbrand , as I am pretty much through the whole code alreadyâŠ
Meanwhile I have two last issues which I do not have an answer for:
if (positionData!=null) {
// do some stuff
for (int index = 0; index < numPanels; index++) {
if (positionData != null) {
@Nullable PositionDatum panel = positionData.get(index);
if (panel != null) {
Now it complains on the second but last line (@Nullable PositionDatum panelâŠ)
Potential null pointer access: this expression has a â@Nullableâ type
WHY? I am checking the âif positionData is nullâ one line before, so it can never be null at that line, can it?
I have the following code
@Nullable Write response = gson.fromJson(...)
if (response!=null) {
and it complains about it âThe variable response cannot be null at this locationâ
Why can it be so sure? It cannot know the code of gson.fromJson(), can it?
(Actually it is try because it returns âPrimitives.wrap(classOfT).cast(object);â)
on the other hand if I do not declare the @Nullable Write it will complain by issuing
Unsafe interpretation of method return type as â@NonNullâ based on substitution âT=@NonNull Writeâ. Declaring type âGsonâ doesnât seem to be designed with null type annotations in mind
I absolutely hate the nullable annotation for too many reasons (and donât want to hijack this thread).
Hard to tell whatâs going on since you are only posting snippets. But if positionData is a non-final instance variable - then the compiler assumes that positionData could potentially change between your first reference and your next one [by some other thread] and thus can potentially be null (regardless if there is no possibility of that happening).
What I do (about 1000 times in the sony plugin) is something like:
That way the compiler knows it canât be null after the if statement.
As for the gson one - weird, I use it all time time (without the @Nullable) and I donât have issues. What Iâm betting is that the variable you are passing into the fromJson is potentially null and itâs complaining about that (might be the same issue as the above).
EDIT: just did a full compile - the GSON one is just a INFO (or warn) message and can be ignored. Only the ERROR oneâs really need addressingâŠ
I will look at the warnings and your recommendations at the weekend. I appreciate your time for answering that and will revert as soon as I had a deeper look into it.
btw, the âdo some stuffâ in the snippet doesnât change anything on positionDataâŠ