On Thu, 11 Mar 2021 17:36:00 GMT, Anthony Scarpino <[email protected]>
wrote:
>> SonarCloud reports:
>> Use "Arrays.equals(array1, array2)" or the "==" operator instead of using
>> the "Object.equals(Object obj)" method.
>>
>> } else if (!src.isDirect() && !dst.isDirect()) {
>> if (!src.isReadOnly()) {
>> // If using the heap, check underlying byte[] address.
>> if (!src.array().equals(dst.array()) ) { // <--- here
>>
>> Additional testing:
>> - [x] Linux x86_64 fastdebug `jdk_security`
>
> Can you explain why the silly way it is written now is any different than
> what you are proposing? When running jshell your proposed change returns the
> same result as the existing code:
>
> one ==> byte[5] { 1, 2, 3, 4, 5 }
> two ==> byte[5] { 1, 2, 3, 4, 5 }
> jshell> one != two
> $9 ==> true
>
> jshell> !one.equals(two)
> $10 ==> true
>
> jshell> one != one
> $11 ==> false
>
> jshell> !one.equals(one)
> $12 ==> false
>
> Is the analysis tool thinking equals() is comparing the contents?
> Can you explain why the silly way it is written now is any different than
> what you are proposing? Is the analysis tool thinking equals() is comparing
> the contents?
See: https://rules.sonarsource.com/java/RSPEC-2159.
Using `equals` raises the question if actual array contents comparisons was
implied (which would require `Arrays.equals`). I know it is not implied here.
Using `==` makes the intent clear. This is why the rule asks to choose either
`==` or `Arrays.equals`. This is the only instance where that rule triggers in
whole OpenJDK code.
>From a micro-optimization perspective, `equals` is a method. This means
>interpreters are would have to do that call. Compilers would hopefully inline
>the call, but then it would still imply the null-check, because NPE should be
>thrown if one side is `null`.
So, why call `equals()`, when `==` is better?
-------------
PR: https://git.openjdk.java.net/jdk/pull/2938
