On 21/06/2019 21:20, Joe Wang wrote:
Full webrev (for the record)
http://cr.openjdk.java.net/~joehw/jdk13/8224157/webrev_02/
A short version of webrev_02 that includes the only files mentioned in
this review:
http://cr.openjdk.java.net/~joehw/jdk13/8224157/webrev_02_short/
Thanks Joe!
The updated files look good.
Thumbs up!
Minor comment - that you might want to propagate upstream:
@Override
- public boolean equals(final Object o1, final Object o2) {
+ public boolean equals( final Object o1, final Object o2 ) {
final JavaClass THIS = (JavaClass) o1;
final JavaClass THAT = (JavaClass) o2;
- return THIS.getClassName().equals(THAT.getClassName());
+ return Objects.equals(THIS.getClassName(),
THAT.getClassName());
}
+
@Override
- public int hashCode(final Object o) {
+ public int hashCode( final Object o ) {
final JavaClass THIS = (JavaClass) o;
return THIS.getClassName().hashCode();
}
I've seen that idiom at a few places: equals was modified to
use Objects.equals(), but hashCode was not.
1. equals will throw if it is passed a null THIS or THAT (is
that intented?)
2. equals seems to think that getClassName() can return null,
whereas hashCode obviously expect that it will be non null.
I cannot say whether that's right or wrong, but I can smell something...
Maybe having a look at the issue that triggered the use of
Objects::equals could shed some light on this. It doesn't seem worse
than what was there before - it's just a bit odd.
best regards,
-- daniel
