You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@commons.apache.org by ol...@apache.org on 2013/09/06 05:49:02 UTC

svn commit: r1520487 - /commons/sandbox/monitoring/trunk/core/src/main/java/org/apache/commons/monitoring/Role.java

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)) {
             return false;
         }



Re: svn commit: r1520487 - /commons/sandbox/monitoring/trunk/core/src/main/java/org/apache/commons/monitoring/Role.java

Posted by sebb <se...@gmail.com>.
On 6 September 2013 11:08, sebb <se...@gmail.com> wrote:
> On 6 September 2013 04:49,  <ol...@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


Re: svn commit: r1520487 - /commons/sandbox/monitoring/trunk/core/src/main/java/org/apache/commons/monitoring/Role.java

Posted by sebb <se...@gmail.com>.
On 6 September 2013 04:49,  <ol...@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);


>          }
>
>

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org