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