SonarQube bugs

[tmrpavan]

Avoid Comparing Integers in Java using == Operator

[tmrpavan]

@aalhossary Can you please review this PR

[josemduarte]

I think auto-unboxing takes care of this in most cases. For instance if I rewrite your first test as:

Integer m = 1;
Integer n = 1;
System.out.println(m==n);

The output is true

[josemduarte]

Interestingly this doesn't work:

Integer m = new Integer(1);
Integer n = new Integer(1);
System.out.println(m==n);

but this works:

Integer m = Integer.valueOf(1);
Integer n = Integer.valueOf(1);
System.out.println(m==n);

[aalhossary]

@josemduarte That is because of the internal implementation of valueOf()
It first tries to find a cached/cachable value of the parameter in a static HashTable of Integer. If found, it returns that statically cached object.
new Integer on the other hand, creates a new object in all cases.
UPDATE: autoboxing uses valueOf() under the hood.

Therefore, checking the equality of two autoboxed wrappers using == is safe within a range (as far as I remember in the range [-127, 128]).

However, the situation in this pull request (and the previous one) is actually about "not equal".

[tmrpavan]

comparison rather than equality comparison. When using the "not equal" operator (!=) to compare two autoboxed Integer values, the result is not guaranteed to be correct, even within the range you mentioned.

This is because the "not equal" operator first unboxes the Integer values into primitive int values and then compares those values. If the two Integer values being compared happen to be in the cached range, they will be the same object reference and thus have the same primitive value. However, if they are not in the cached range, they will be different object references even if they have the same primitive value, and the "not equal" comparison will return false when it should have returned true.

To avoid this issue, it's recommended to use the equals() method for comparing two autoboxed Integer values instead of the "not equal" operator, as it compares the values of the objects rather than their references.

[tmrpavan]

@aalhossary See the implementation on Objects.equals();

It first compares the objects with == operator.
1-> If bother are in cached range if both are same the first expression return true.
2-> If any one of the object is not in cached range if first expression return is flase for some case that case only it is going to check the values.

I hope using equals method safe.

public static boolean equals(Object a, Object b) {
return (a == b) || (a != null && a.equals(b));
}
I
equals ion Integer class
public boolean equals(Object obj) {
if (obj instanceof Integer) {
return value == ((Integer)obj).intValue();
}
return false;
}

[tmrpavan]

@aalhossary
Is there any side effects using Objects.equals(). As per you previous post -> #1052 (comment) it gives expected results for all the scenarios.

[aalhossary]

@tmrpavan using Objects.equals() instead of Integer.equals() (and instead of == of course) does the right job.

[aalhossary]

@josemduarte Now both this PR and #1052 address the same issue (i.e. duplicates).
What shall we do? close the old one and review the new one, or close the new one and ask the old one OP for modifications?

[tmrpavan]

@aalhossary Any update on this

[josemduarte]

Now both this PR and #1052 address the same issue (i.e. duplicates).
What shall we do? close the old one and review the new one, or close the new one and ask the old one OP for modifications?

Since #1052 has gone quiet (specially from the PR creator side). Let's close that one in favour of this. Still we'll want to review this PR of course.

[aalhossary]

Please see my comments on your modifications

[tmrpavan]

I am working on this

[tmrpavan]

@aalhossary Modified as per the comments

[aalhossary]

LGTM. Thank you!