On 10/03/2013 02:03 PM, roger riggs wrote:
Hi Sherman,
On 10/3/2013 2:38 PM, Xueming Shen wrote:
(1) until(Temporal endExclusive, TEmporalUnit unit)
-> Shouldn't we invoke requireNonNull() for unit before invoking
unit.between(...)?
There is no doubt that invoking unit.between() will cause an NPE if unit is
null.
Adding requireNonNull will only improve the error message and add bulk to the
code.
Most of the use cases of requireNonNull here are "no doubt that a NPE will be
triggered"
and the only benefit of having them is to improve the err msg and there for add
bulk to
the code. What I'm saying here is the consistency.
(3) We have the requireNonNull check pattern below in most places
if (X instanceof Y) { ... }
Objects.requireNonNull(X, "x");
Isn't that just useless, instanceof only returns true for non-null argument.
but the plus/minus(TemporalAmount amountToSubtract) appears to be
Objects.requireNonNull(...)
if (X instanceof ...) {...}
This form would identify the null as coming from the parameter but the null
check would
be performed twice (unless HS optimized)
I may not have stated my observation clearly enough. In most of the cases
the code inside {...} returned. My guess of intent of the original code is to
optimize the case of non-null parameter AND the X is the instance of Y, which
should be normal use scenario. But the plus/minus implementation appears
not try to optimize this (null always triggers false for instanceof) for case
that
the amountToAdd is indeed a Period. This is also related my argument in (1),
why we check requireNonNull here but not in (1). Well, not a big deal though.
-Sherman
