[MNG-8510] Avoid duplicate Utils classes#2173
Conversation
| import java.util.stream.Collectors; | ||
|
|
||
| class CoreUtils { | ||
| public static <T> T nonNull(T t) { |
There was a problem hiding this comment.
Or should we use Objects.requireNonNull(T) instead? The exception is not the same (NullPointerException instead of IllegalArgumentException), but the former became the standard usage in the JDK API. It is also consistent with the code that just rely on implicit null-check, especially since Java 14 automatically generates an exception message with the name of the parameter that was null. Finally, Objects.requireNonNull(T) as some JVM-level optimization (the @ForceInline annotation) that we can't reproduce in our own null-check method.
There was a problem hiding this comment.
Note: the automatically generated message cited in above comment is for implicit null-checks, not for the exception thrown by Objects.requireNonNull(T). I would propose that when a method uses the parameter immediately like below:
public void foo(String biz) {
if (biz.equals("foo")) {
}
}If biz is null, Java 14 and latter while automatically generates a message saying "Cannot invoke "String.equals(Object)" because "biz" is null". Therefore, it may be better (compared to no message at all) to not invoke a method such as nonNull if we accept that the exception is NullPointerException. For cases where the implicit null check would be too late, the Objects.requireNonNull methods throw an exception of type consistent with the implicit null checks.
There was a problem hiding this comment.
Yes, agreed, Eliotte raised the same issue a while ago. Though the point of this PR was just to avoid the Utils class name clash, so we can create a subsequent PR to remove those custom methods.
There was a problem hiding this comment.
Also agreed. This PR is an improvement by itself, so merge it and then fix that.
|
Resolve #9926 |
JIRA issue: MNG-8510