support null data in nodes #4380
Conversation
|
Looks good. 2 notes:
Also: I will create another issue for preventing passing |
| // we need a stable hash code for _value == null | ||
| return 0; | ||
| } | ||
| return Double.valueOf(doubleValue()).hashCode(); |
There was a problem hiding this comment.
huh? Oh, that's how code already was. Ok. :)
There was a problem hiding this comment.
But not sure why not use BigDecimal.hashCode(), fwtw.
Maybe something else to change in.... 2.17 I think.
There was a problem hiding this comment.
The pre-existing equals method is odd as well. It would be easier to use .equals on the BigDecimal instances.
There was a problem hiding this comment.
compareTo and the special hashCode are necessary here because BigDecimal equality is weird wrt scaling. 1.9 and 1.90 are considered unequal by BigDecimal but equal by this DecimalNode impl.
There was a problem hiding this comment.
Ok; will then leave that as-is.
pjfanning
force-pushed
the
more-null-node-equals
branch
from
d4753f0 to
39305f9
Compare
@cowtowncoder retargeted to 2.15 branch |
|
Thank you @pjfanning ! I merged; did minor clean up for 2.17, some more for 3.0 and planning to do #4381 tomorrow, to prevent wrapping of |
relates to #4379
It may not be expected that these node types wrap nulls but our constructors let you create instances that wrap nulls. So unfortunately, we need to code for them.
There is an argument that we should fix the constructors to not allow null inputs.