On 8/29/13 4:11 PM, shanliang wrote:
Here is the new version:
http://cr.openjdk.java.net/~sjiang/jdk-8023669/01/
Indeed, calling Objects.hash(Object ...) is a good idea, which
simplifies the code.
I used also Arrays.hashCode() to simplify the code, now the fix likes
really simple.
I have passed JCK tests, unit tests of management are passed too in my
desk.
toString() worked perfectly in the test, nothing to fix.
Shanliang
Hi Shanliang,
This looks good! Thumbs up!
-- daniel
Daniel Fuchs wrote:
On 8/29/13 9:34 AM, shanliang wrote:
Hi,
Please review following fix, it addresses the issue only in the method
"hashCode":
bug: https://bugs.openjdk.java.net/browse/JDK-8023669
webrev: http://cr.openjdk.java.net/~sjiang/jdk-8023669/00/
Thanks,
Shanliang
Hi Shanliang,
<http://cr.openjdk.java.net/~sjiang/jdk-8023669/00/src/share/classes/javax/management/MBeanAttributeInfo.java.frames.html>
I suggest to simplify this by calling:
public int hashCode() {
return Objects.hash(getName(), getType());
}
(see
<http://docs.oracle.com/javase/7/docs/api/java/util/Objects.html#hash%28java.lang.Object...%29>)
<http://cr.openjdk.java.net/~sjiang/jdk-8023669/00/src/share/classes/javax/management/MBeanConstructorInfo.java.frames.html>
int hash = getName() == null ? 10 : getName().hashCode();
could be replaced by:
int hash = Objects.hashCode(getName());
Generally - and that stands for the other files you modified, you can
simplify things by replacing x.hashCode() with Objects.hashCode(x)
whenever there's the possibility that x can be null.
Note however that Objects is an API @since JDK 7 - so if you intend
to backport this fix to 6 & 5 you will need to alter your changeset
when backporting it.
MBeanOperationInfo.java, MBeanParameterInfo.java: I suggest
to use Objects.hash(...);
best regards,
-- daniel
BTW: one more question: you're also testing toString() in the test
and that's good - but are there any toString() that will require
fixing?
