Hi John,
Thanks for the followup.
It is a subtle issue and seems to hinge on the relative priority of
consistency in naming
and the degree that historical artifacts ('mistakes') constrain
contemporary patterns.
If we were re-writing history:
boolean nonNull(obj) could/should have been named 'isNonNull'.
T nonNull(T, String) could/should have been T nonNullThrows(T,
Supplier<Exception>)
The notion that the prefix "nonNull" is already taken and the
anti-pattern, seem a lower priority.
The prefix can be understood to introduce a family of similar not
necessarily identical semantics.
Most method names try to describe the action taken, not the pre- or
post-conditions.
I prefer shorter names and was ready to capitulate until I ran into this:
T requireNonNull(T)
T requireNonNull(T, String)
T requireNonNull(T, T) -- now is ambiguous with the
pre-existing method
So just add another word to disambiguate:
T requireNonNullElse(T, T) -- now 23 characters counting
required punctuation
without room
for the return value or arguments.
T requireNonNullElseGet(T, Supplier<T>)
The shorter names were intended to be easier to read and save the space
on the line
for the arguments which are the important parts of the expression.
The longer names disambiguate adequately but add to the bulk of the code.
I see maturing systems end up being weighed down by the seemingly necessary
qualification.
So in face of the tradeoffs, what were you proposing again?
Roger
On 10/28/15 5:44 PM, John Rose wrote:
On Oct 9, 2015, at 8:46 AM, Roger Riggs <roger.ri...@oracle.com> wrote:
The primary purpose of the existing 'requireNonNull' methods is to
check for null
and throw an exception. It is used for explicit testing and producing
a cogent exception.
Returning the value was a secondary benefit that allowed it to be
used in fluent expressions.
As I noted before, the stated purpose of "requireNonNull" can be
slightly widened to check for null, and either throw an exception or
return an alternative non-null value. This is a compatible change.
Meanwhile, I just realized (after working on code in an IDE) that we
already have this API point:
boolean nonNull(Object x) { return x != null; }
The new proposed name "nonNullElse" would seem on first glance to be a
variation on that API point, but it's very different, since it returns
T instead of boolean. It seems to me that the prefix "nonNull" is
already taken, and we have added an incompatible method to that family.
The new API point "nonNullElse" is much closer in meaning to
"requireNonNull", as noted before:
T nonNullElse(Object x, Object y) { return x != null ? x : y; }
T requireNonNull(Object x) { return x != null ? x : throw … }
Having different overloadings of the same name confuse T and a
primitive type (boolean) is a known anti-pattern. (Cf.
List.remove(int|T).) In this case, the names differ but "nonNull" and
"nonNullElse" are going to be viewed together often as a group. They
are presented together in an IDE completer. When I complete a name, I
usually have in mind what I want it to return. Therefore, the
completion of "Objects.notN*" will show me a boolean-valued and a
T-valued result, one of which is not what I want. Meanwhile, if I
were to complete "Objects.req*" I would get a choicer of T-valued
operators, but not one particular one that I might want ("notNullElse").
As I said to you verbally, I'm not going to "die on this hill", but I
do think the name chosen has some disadvantages that could have been
avoided with a very slight mental shift about require. I.e.,
"requireNonNull*" requires its *return value* to be not-null, so if
its *argument* is null, then an adjustment is made (throw, select
alternate value, get-from-supplier).
— John
