On Mon, 26 Jan 2026 13:18:13 GMT, Jaikiran Pai <[email protected]> wrote:

>> Eirik Bjørsnøs has updated the pull request incrementally with one 
>> additional commit since the last revision:
>> 
>>   Use "list" instead of "queue" when referring to expandedPath which is 
>> processed FIFO and as such is not a queue
>
> 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));

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/29288#discussion_r2727618873

Reply via email to