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.
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
