On Mon, 26 Jan 2026 13:32:20 GMT, Eirik Bjørsnøs <[email protected]> wrote:
>> src/java.base/share/classes/jdk/internal/loader/URLClassPath.java line 139:
>>
>>> 137: public URLClassPath(URL[] urls,
>>> 138: URLStreamHandlerFactory factory) {
>>> 139: // Reject null URLs
>>
>> Nit - I think it would be better to clarify that it's intentional to
>> (continue) to reject null element in the array. So may be reword this
>> comment to "Reject null URL array or any null element in the array"?
>
> Yeah, the issue here is that `ArrayList` is null friendly while `ArrayDeque`
> was hostile.
>
> So since there is no such thing as`Arrays::requireNonNullElements`, I used
> `List.of`, (as suggested by @stuart-marks in JDK-8301042).
>
> But that seemed non-obvious and brittle, hence the addition of the comment.
>
> What would you think about being explicit in the null check, such that the
> code documents itself:
>
>
> // Reject null URL array or any null element in the array
> Objects.requireNonNull(urls);
> for (URL url : urls) {
> Objects.requireNonNull(urls);
> }
>
> this.searchPath = new ArrayList<>(Arrays.asList(urls));
I think using `List.of()` is the right thing to do, since its behaviour is
specified. A iteration as proposed here would very likely be refactored later
into a `List.of()` as a clean up, when someone spots this code.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/29288#discussion_r2727646276
