RFR 8141409 Arrays.equals accepting a Comparator

[Paul Sandoz]

Hi Roger,

I want to keep this review focused and consistent with the existing methods that were previously reviewed.


> On 12 Nov 2015, at 17:42, Roger Riggs <Roger.Riggs at Oracle.com> wrote:
> 
> Hi Paul,
> 
> - A minor point but in the argument names, its a bit inconsistent between using 'b' and 'a2'.
> I would have use 'a', and 'b' to be more consistent.  (and yes the current methods show the same inconsistency)
> 

Perhaps, but i don’t want to do that right now.


> - The @return(s) should have an 'otherwise {@code false};  otherwise maybe it is allowed to return true.
> 

The current approach is consistent with the existing equals methods.


> - In the implementation, I generally prefer the form of Objects.requireNonNull(o, "name") so the exception
> identifies the offending argument.
> 
> - I thought the coding style has spaces around operators (like ==).
> 

Same as in other equals methods.


> - The non-range checked version of equals should have the same behavior as:
>   equals(a, 0, a.length, a2, 0, a2.length).
> 
> The range checked version should include the sentence:
> 
> "Also, two array references are  considered equal if both are {@code null}.”
> 

Whats the sub-range of a null array reference?

See the existing sub-range equals methods. This was discussed in the original review and i don’t want to reopen that decision right now.


> - The implementation will need to check for nulls before the rangeChecks.
> 

The access to the length will induce an NPE. See other methods in arrays (such as binarySearch)

Paul.