Frederic,
Looks good. And I like the refactoring.
A question - does the Descriptor field have the same potential for being null?
e.g.
in MBeanFeatureInfo.java new line 160 - do you need to check getDescriptor() ==
null as well?
in MBeanConstructorinfo.java line 197, do you need the alternative null
checking?
MBeanAttributeInfo.java line 293?
(there may be other lines, I didn't do a thorough search)
thanks,
Karen
On Apr 13, 2012, at 9:01 AM, Staffan Larsen wrote:
> This is much easier to read!
>
> In MBeanFeatureInfo.hasSameName() this check is not needed:
>
> 274 if(info.getName() == null) {
> 275 return false;
> 276 }
>
> since String.equals() with a null argument works as expected (return false).
> The same is true for the implementation in hasSameDescription().
>
> When calling hasSameName() and hasSameDescription() the "this." is not
> needed, but perhaps using it makes the intent clearer?
>
> Thanks,
> /Staffan
>
>
>
>
> On 13 apr 2012, at 14:45, Frederic Parain wrote:
>
>> Thanks Staffan and Dmitry for your feedback,
>> here's a new webrev with a refactored code for
>> name and descriptions fields which are common to
>> most MBean*Info classes:
>>
>> http://cr.openjdk.java.net/~fparain/7148497/webrev.01/
>>
>> Thanks,
>>
>> Fred
>>
>> On 4/12/12 8:18 AM, Dmitry Samersoff wrote:
>>> Frederic,
>>>
>>> I'm second to Staffan.
>>>
>>> Looks good to me but would prefer to have it refactored, e.g. create a
>>> method equalOrNull();
>>>
>>> -Dmitry
>>>
>>>
>>> On 2012-04-10 18:33, Staffan Larsen wrote:
>>>> Looks good! (Although I wish there was a better way to write code like
>>>> this).
>>>>
>>>> /Staffan
>>>>
>>>> On 10 apr 2012, at 15:50, Frederic Parain wrote:
>>>>
>>>>> Greetings,
>>>>>
>>>>> This is a request for review for CR
>>>>> 7148497: javax.management.MBeanAttributeInfo.hashCode throws
>>>>> NullPointerException
>>>>> http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=7148497
>>>>>
>>>>> Even if the initial CR only refers to MBeanAttributeInfo.hashCode(),
>>>>> the problem is not limited to the MBeanAttributeInfo.hashCode(), but
>>>>> is common to all javax.management.MBean*Info classes:
>>>>> MBeanAttributeInfo, MBeanConstructorInfo, MBeanFeatureInfo,
>>>>> MBeanInfo, MBeanNotificationInfo, MBeanOperationInfo and
>>>>> MBeanParameterInfo.
>>>>>
>>>>> The root cause of the problem is that these classes can be
>>>>> instantiated with some null fields (mainly, but not limited to, the
>>>>> name and type fields), and these fields are dereferenced
>>>>> unconditionally in some methods. Unconditional dereferencing is used
>>>>> in the hashCode method but also in the equals() method.
>>>>>
>>>>> The proposed fix improves implementation of equals() and hashCode()
>>>>> method to handle cases where some fields are null without throwing
>>>>> a NPE.
>>>>>
>>>>> Webrev:
>>>>> http://cr.openjdk.java.net/~fparain/7148497/webrev.00/
>>>>>
>>>>> Thanks,
>>>>>
>>>>> Fred
>>>>>
>>>>> --
>>>>> Frederic Parain - Oracle
>>>>> Grenoble Engineering Center - France
>>>>> Phone: +33 4 76 18 81 17
>>>>> Email: frederic.par...@oracle.com
>>>>>
>>>>
>>>
>>>
>>
>> --
>> Frederic Parain - Oracle
>> Grenoble Engineering Center - France
>> Phone: +33 4 76 18 81 17
>> Email: frederic.par...@oracle.com
>>
>
