Karen,
As far as I can tell, getDescriptor() never returns null.
In the javax.management package, getDescriptor() calls
ImmutableDescriptor.nonNullDescriptor(descriptor).clone()
which returns a reference to a singleton empty Descriptor
instance if the descriptor field is null.
The getDescriptor() method is redefined in some classes of
the javax.management.modelmbean package, but these implementations
never return null either.
So, unless I missed some code, it is safe to use getDescriptor()
without null check.
Fred
On 4/13/12 3:54 PM, Karen Kinnear wrote:
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
--
Frederic Parain - Oracle
Grenoble Engineering Center - France
Phone: +33 4 76 18 81 17
Email: frederic.par...@oracle.com
