uschindler edited a comment on pull request #586:
URL: https://github.com/apache/lucene/pull/586#issuecomment-1006389841


   Hi Robert,
   
   the change is actually easy and I can also do this in this PR. What I am 
behind is:
   
   - I don't want to enforce requireNonNull to always have a message, this is 
really up to the use case. In tests or internal code it makes no sense, because 
if something happens you need the stacktrace and debugging anyways. So 
forbiddenapis is not my intention. I just wanted those at places for public APIs
   - For me as a user of a public API, it would really be good to see from the 
message which parameter I passed was wrong - especially if there are multiples 
(it is a mess to call amthod with 5 parameters and then get NPE). With 
IllegalArgumentException we also always give an explanation like "parameter foo 
must bei >=1". The stack trace wo message always requires you to actually look 
into the source code of the method you called.
   
   Inside JDK's code it is not always used with message (so it is mixed usage, 
depends on the reviewer and API history), so I agree you can have both! But 
when it is used, the name of the parameter is mostly given (or some message 
referring to the name of the parameter): e.g. 
https://github.com/openjdk/jdk/blob/2dbb936da91a875a5a88da5d4c0b4956b9c6368f/src/java.base/share/classes/java/nio/channels/Channels.java#L106
   
   BTW, if you want the "nice automatic Java message", replace 
`Objects.requireNonNull(foo);` by `foo.getClass()` (this method is always 
available and calling it is cheap; if foo is null, you get the nice 
JDK-generated message -- downside: it talks then about "calling getClass").
   
   I think the JDK people should improve also Objects.requireNonNull to look 
into its own stack trace (Stackwalker also returns bytecode index, so 
theoretically you can do the same to figure out how the argument was pushed to 
stack). Maybe I will write a PR/JEP for them. The algorithm looks reuseable.
   
   If you dont want to do it, I offer to do this for the cases used here! At 
least I would like to have useful exception messages for all newly introduced 
IAE and NPE.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org
For additional commands, e-mail: issues-h...@lucene.apache.org

Reply via email to