API REVIEW REQUEST: Public API for Backgrounds and Borders on Region
richard.bair at oracle.com
Mon Aug 27 07:21:07 PDT 2012
Thanks for taking a look!
> One remark though, for hashCode, 0 is a valid value which could easily occur in normal operation (reminds me of a Unit Test that would rarely fail because it checked hashCode != 0). If it is zero, some of the classes will recompute it every time (or they might recompute it until it is non-zero, although that may not happen if all the classes they depend on are immutable). I think it is better computed right at the start (in the constructor) as they are immutable anyway -- that also makes it guaranteed thread-safe.
I'm not really concerned about the case where hashCode == 0, because it is unlikely (?) to happen in practice and the worst case is that the computation takes longer rather than returning the wrong result. That being said, I do like the suggestion of computing the hash code up front because these classes are all going to end up in a hash table pretty much right off the bat (in the NGRegion implementation, the Background is a key associated with a cached representation of the region which we can than blit quickly rather than redrawing each layer). So computing the hash ahead of time would be useful. Also, because the classes are immutable I put the check in equals to fail if the hash of this object != the hash of the other object, but I couldn't read the field directly but had to invoke the hashCode() method since it might not have been computed yet. Putting the hash computation in the constructor means I can just compare fields here.
More information about the openjfx-dev