Are there any bugs in the database about the fact that many of these
ColorModel subclasses implement equals without hashcode? They really
should both be implemented, but since ColorModel implements the method
based on its tests, they are technically covered by the equals/hashCode
contract - it's just not a very optimal implementation of the contract.
For now, it would be good to implement hashCode() on ICM now that you
are creating an equals() method for it.
Also, ColorModel.hashCode() is a poor implementation. It doesn't use
the paradigms recommended by Effective Java and looks like it produces
poor hashes as a result (just in the first two elements of the hashCode
I see a collision due to poor choice of numbers).
With respect to this particular equals() override I think it looks fine,
though the use of the isValid() method reduces its performance for a
couple of reasons:
- it retests the index for the range [0,map size] which we already know
is valid
- if the validBits is null, there is no need to even do the test.
This might be faster for the very common case:
if (validBits == null) {
if (cm.validBits != null) return false;
// loop just comparing rgb[] values
} else {
if (cm.validBits == null) return false;
// loop, testing rgb[] values and
// corresponding validBits indices directly
}
Note that in the constructor we set validBits to null if it is "true"
for all valid indices so if one of the cmaps has it null and the other
does not, then that is a very good indicator that their valid indices
don't match.
On a more minor note, I don't like the indentation of the if statement,
I'd move the "{" brace to a line of its own indented the same as its
closing "}" to make the body of the if stand out. It isn't strictly the
Java coding guidelines, but it does match a bunch of the other 2D code -
sort of a local exception to the coding style.
I'd also add a few test cases that test that 2 separately and
identically constructed ICM instances are equals() == true, tested with
one of each of the constructors...
...jim
On 4/6/2016 4:47 AM, Jayathirth D V wrote:
Hi,
_Please review the following fix in JDK9:_
Bug : https://bugs.openjdk.java.net/browse/JDK-7107905
Webrev : http://cr.openjdk.java.net/~jdv/7107905/webrev.00/
Issue : When we compare two IndexColorModel objects with different
ColorMap(or any other property specific to IndexColorModel) using
equals() method it returns true.
Root cause : There is no override equals() method specific to
IndexColorModel and it uses super class equals() which is ColorModel and
it doesn’t verify equality for IndexColorModel specific properties.
Solution : Add override equals() method for IndexColorModel which
verifies its unique properties for equality.
Thanks,
Jay
