Hi Phil,

Thank you for your review! That's a valid point!

Please see the updated webrev:
http://cr.openjdk.java.net/~aivanov/8222108/webrev.02/

I implemented a custom comparator which handles null values in the printer list.

However, I wonder if the list of printers can ever contain a null value. The method refreshServices() does not check if printers[p] is null.

On 03/07/2019 00:13, Philip Race wrote:
I thought we had the checks for null in doCompare there for a reason.
Arrays.sort won't be very happy with a null element.

You said in the first review email of the v0 webrev :

> Arrays.equals() accepts null parameters and null elements in the arrays and always returns the correct result.

but that webrev didn't use Arrays.sort and that requirement seems to have been forgotten
when adding it.

 public class Sort {
  public static void main(String[] args) {
    String[] a1 = { "a", null, "a" };
    java.util.Arrays.sort(a1);
  }
}

java Sort
Exception in thread "main" java.lang.NullPointerException
    at java.util.ComparableTimSort.countRunAndMakeAscending(ComparableTimSort.java:320)
    at java.util.ComparableTimSort.sort(ComparableTimSort.java:188)
    at java.util.Arrays.sort(Arrays.java:1246)
    at Sort.main(Sort.java:4)


-phil.

On 7/2/19, 5:18 AM, Alexey Ivanov wrote:
Are there any other comments?
Other volunteers for reviewing?

Thanks in advance.

On 25/06/2019 20:17, Alexey Ivanov wrote:
Please see the updated webrev:
http://cr.openjdk.java.net/~aivanov/8222108/webrev.01/

<SNIP>
--
Regards,
Alexey

Reply via email to