jmx-dev Codereview request for JDK-8023669: MBean*Info.hashCode : NPE

[shanliang]

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

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?
>
>