[jira] [Comment Edited] (MNG-8509) org.apache.maven.internal.impl.Utils.nonNull should be removed

["Guillaume Nodet (Jira)" <ji...@apache.org>]

    [ https://issues.apache.org/jira/browse/MNG-8509?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17912098#comment-17912098 ] 

Guillaume Nodet edited comment on MNG-8509 at 1/10/25 7:42 PM:
---------------------------------------------------------------

Fwiw, I don't think they throw the wrong type of exceptions.
{{NullPointerException}} is defined as {{Thrown when an application attempts to use null in a case where an object is required.}}
{{IllegalArgumentException}} is defined as {{Thrown to indicate that a method has been passed an illegal or inappropriate argument.}}
I do think that {{throw new IllegalArgumentException("param cannot be null")}} is more meaningful than {{throw new NullPointerException("param")}} because {{IllegalArgumentException}} is more specific as it's not thrown implicitly by the JVM.  Ideally, in the case where we check the value of arguments, we'd have a {{NullArgumentException}} which would be a subclass of {{IllegalArgumentException}}.

====

Wrt to merging {{Utils}} classes, they are completely unrelated, so I don't think it would make much sense.  

The one from {{org.apache.maven.internal.impl}} is package protected, so imho, it's fine.  

The one {{org.apache.maven.di.impl}} contains two methods which are used only from {{org.apache.maven.di.Key}} class.  It can be moved to that package and made package protected too (again, I don't like too much code in an API package, but...)

The last one is {{org.apache.maven.cling.invoker}} should be private to {{maven-cli}} module, but that is not possible, as we don't use JPMS.  
Fwiw, I think the {{createInterpolator}} in that class should be migrated to use {{org.apache.maven.internal.impl.model.DefaultInterpolator}} instead.


was (Author: gnt):
Fwiw, I don't think they throw the wrong type of exceptions.
{{NullPointerException}} is defined as {{Thrown when an application attempts to use null in a case where an object is required.}}
{{IllegalArgumentException}} is defined as {{Thrown to indicate that a method has been passed an illegal or inappropriate argument.}}
I do think that {{throw new IllegalArgumentException("param cannot be null")}} is more meaningful than {{throw new NullPointerException("param")}} because {{IllegalArgumentException}} is more specific as it's not thrown implicitly by the JVM.  Ideally, in the case where we check the value of arguments, we'd have a {{NullArgumentException}} which would be a subclass of {{IllegalArgumentException}}.


> org.apache.maven.internal.impl.Utils.nonNull should be removed
> --------------------------------------------------------------
>
>                 Key: MNG-8509
>                 URL: https://issues.apache.org/jira/browse/MNG-8509
>             Project: Maven
>          Issue Type: Improvement
>            Reporter: Elliotte Rusty Harold
>            Priority: Major
>
> Aside from throwing the wrong exception type, this just reinvents Objects.requireNonNull that we've had since Java 8.
> Also we have 3 different classes named Utils at HEAD and we should either combine, rename, or eliminate them as appropriate.



--
This message was sent by Atlassian Jira
(v8.20.10#820010)