On 6 September 2013 11:08, sebb <[email protected]> wrote: > On 6 September 2013 04:49, <[email protected]> wrote: >> Author: olamy >> Date: Fri Sep 6 03:49:02 2013 >> New Revision: 1520487 >> >> URL: http://svn.apache.org/r1520487 >> Log: >> simplify code >> >> Modified: >> >> commons/sandbox/monitoring/trunk/core/src/main/java/org/apache/commons/monitoring/Role.java >> >> Modified: >> commons/sandbox/monitoring/trunk/core/src/main/java/org/apache/commons/monitoring/Role.java >> URL: >> http://svn.apache.org/viewvc/commons/sandbox/monitoring/trunk/core/src/main/java/org/apache/commons/monitoring/Role.java?rev=1520487&r1=1520486&r2=1520487&view=diff >> ============================================================================== >> --- >> commons/sandbox/monitoring/trunk/core/src/main/java/org/apache/commons/monitoring/Role.java >> (original) >> +++ >> commons/sandbox/monitoring/trunk/core/src/main/java/org/apache/commons/monitoring/Role.java >> Fri Sep 6 03:49:02 2013 >> @@ -83,9 +83,8 @@ public class Role implements Comparable< >> if (getClass() != obj.getClass()) >> return false; >> final Role other = (Role) obj; >> - if (name == null) { >> - if (other.name != null) >> - return false; >> + if (name == null && other.name != null) { >> + return false; >> } else if (!name.equals(other.name)) { // <== NPE? >> return false; > > There's a potential NPE if name==null and other.name==null. > > Since anything.equals(null) should always return false, I think you > can just write: > > if (name == null) { > return false; > } > return name.equals(other.name); >
Or drop the null check entirely, as properly constructed instances cannot have a null name. The class is not serialisable, so AFAIK the only way to create it is via the ctor which rejects null name. BTW, the name and unit fields should be final. > >> } >> >> --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
