On Tue, Feb 14, 2017 at 12:56 AM, David Holmes <david.hol...@oracle.com> wrote: > On 14/02/2017 2:57 AM, Alan Bateman wrote: >> >> On 09/02/2017 16:18, Volker Simonis wrote: >> >>> : >>> >>> I really think that the simplest and most natural fix for this problem >>> is to simply check for a null Supplier argument and create the NPE >>> with an explicit null message in that case (as outlined in the >>> webrev). This: >>> - makes the two requireNonNul() overloads for String and Supplier >>> behave the same (which I think was the initial intention). >>> - doesn't require documentation changes >>> - makes it possible to write a simple and conforming TCK test >>> >>> If we can't agree on this quickly, I suggest to close the issue as >>> "will-not-fix" and leave it untested by the TCK. >>> >> If I read JDK-8033909 correctly then it's a request for the javadoc to >> specify the expected behavior when called with a messageSupplier of >> null. I think Joe said at one point that he deliberately didn't specify >> this. If that remains the position then the issue can be closed as WNF >> so that we don't waste any more discussion on it. > > > I think the fact this method is intended to throw a NPE if the target object > is null, complicates the "normal" position of "if any arg is null we throw > NPE", and so requires explicitly being addressed. I would address this by > making the supplier be optional, so that it is only used if non-null and so > can itself never be the source of the generated NPE. That would then lead > naturally to the implementation Volker proposes as being the only correct > implementation. >
Thanks David! Finally somebody who understands me :) As this seems a very controversial topic, I'll wait for a second positive review before I'll push this. Regards, Volker > David > ----- > > >> If I read your mail and patch correctly then it's more about what the >> NPE message is. I agree the exception you pasted in from the SAP build >> is difficult to read, in which case your patch makes it easier to read. >> That make sense and maybe just push that with a separate issue. >> >> -Alan
