On Thu, 6 Mar 2025 20:01:47 GMT, Andrey Turbanov <aturba...@openjdk.org> wrote:
>> src/java.base/share/classes/java/lang/String.java line 3649:
>>
>>> 3647: Iterable<? extends CharSequence> elements) {
>>> 3648: Objects.requireNonNull(delimiter);
>>> 3649: Objects.requireNonNull(elements);
>>
>> Hello Andrey, I have a different opinion about this. I think removing
>> existing calls to `Objects.requireNonNull()` in favour of implicit null
>> check (some times too far away in the code) isn't useful.
>>
>> You noted that the implicit `NullPointerException` stacktrace are more user
>> friendly now. I think you can get similar level of useful information from
>> `Objects.requireNonNull()` too. In these two specific cases, what you could
>> perhaps do is change those calls to even pass the `message` so that it's
>> clear what parameter is `null`. Something like:
>>
>> Objects.requireNonNull(delimiter, "delimiter");
>> Objects.requireNonNull(elements, "elements");
>
> Should we add it to other overloads of `join` then?
I agree that the easier to read explicit method calls to validate non-nullness
should be left in place.
> Hello Andrey, I have a different opinion about this. I think removing
> existing calls to `Objects.requireNonNull()` in favour of implicit null check
> (some times too far away in the code) isn't useful.
>
> You noted that the implicit `NullPointerException` stacktrace are more user
> friendly now. I think you can get similar level of useful information from
> `Objects.requireNonNull()` too. In these two specific cases, what you could
> perhaps do is change those calls to even pass the `message` so that it's
> clear what parameter is `null`. Something like:
>
> ```java
> Objects.requireNonNull(delimiter, "delimiter");
> Objects.requireNonNull(elements, "elements");
> ```
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/23710#discussion_r1984036994
