commons-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From sebb <seb...@gmail.com>
Subject Re: svn commit: r1520487 - /commons/sandbox/monitoring/trunk/core/src/main/java/org/apache/commons/monitoring/Role.java
Date Fri, 06 Sep 2013 10:52:44 GMT
On 6 September 2013 11:08, sebb <sebbaz@gmail.com> wrote:
> On 6 September 2013 04:49,  <olamy@apache.org> 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: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org


Mime
View raw message