Repository: incubator-reef
Updated Branches:
  refs/heads/master 0d9dc4f47 -> 19809be3c


[REEF-779] Implement both null and type checking in equals functions

This PR prevents potential NPEs and improves performance in equals functions.

JIRA:
  [REEF-779](https://issues.apache.org/jira/browse/REEF-779)

Pull Request:
  Closes #518


Project: http://git-wip-us.apache.org/repos/asf/incubator-reef/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-reef/commit/19809be3
Tree: http://git-wip-us.apache.org/repos/asf/incubator-reef/tree/19809be3
Diff: http://git-wip-us.apache.org/repos/asf/incubator-reef/diff/19809be3

Branch: refs/heads/master
Commit: 19809be3cc4691a29e94c826c4747ba043f8ea4f
Parents: 0d9dc4f
Author: Dongjoon Hyun <[email protected]>
Authored: Wed Sep 23 15:10:16 2015 +0900
Committer: Andrew Chung <[email protected]>
Committed: Wed Sep 23 11:00:19 2015 -0700

----------------------------------------------------------------------
 .../reef/io/network/util/StringIdentifier.java    |  6 ++++++
 .../apache/reef/tang/implementation/TangImpl.java | 18 ++++++++++--------
 .../tang/implementation/types/AbstractNode.java   |  8 ++++----
 .../implementation/types/ConstructorArgImpl.java  |  6 ++++++
 .../implementation/types/ConstructorDefImpl.java  |  6 ++++++
 .../wake/remote/impl/SocketRemoteIdentifier.java  |  6 ++++++
 .../org/apache/reef/wake/remote/impl/Tuple2.java  |  6 ++++++
 .../wake/test/remote/TestRemoteIdentifier.java    |  6 ++++++
 8 files changed, 50 insertions(+), 12 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-reef/blob/19809be3/lang/java/reef-io/src/main/java/org/apache/reef/io/network/util/StringIdentifier.java
----------------------------------------------------------------------
diff --git 
a/lang/java/reef-io/src/main/java/org/apache/reef/io/network/util/StringIdentifier.java
 
b/lang/java/reef-io/src/main/java/org/apache/reef/io/network/util/StringIdentifier.java
index c2de9d0..f558110 100644
--- 
a/lang/java/reef-io/src/main/java/org/apache/reef/io/network/util/StringIdentifier.java
+++ 
b/lang/java/reef-io/src/main/java/org/apache/reef/io/network/util/StringIdentifier.java
@@ -53,6 +53,12 @@ public class StringIdentifier implements 
ComparableIdentifier {
    * @return true if the object is the same as the object argument; false, 
otherwise
    */
   public boolean equals(final Object o) {
+    if (this == o) {
+      return true;
+    }
+    if (o == null || getClass() != o.getClass()) {
+      return false;
+    }
     return str.equals(((StringIdentifier) o).toString());
   }
 

http://git-wip-us.apache.org/repos/asf/incubator-reef/blob/19809be3/lang/java/reef-tang/tang/src/main/java/org/apache/reef/tang/implementation/TangImpl.java
----------------------------------------------------------------------
diff --git 
a/lang/java/reef-tang/tang/src/main/java/org/apache/reef/tang/implementation/TangImpl.java
 
b/lang/java/reef-tang/tang/src/main/java/org/apache/reef/tang/implementation/TangImpl.java
index c9eb1b3..81509a5 100644
--- 
a/lang/java/reef-tang/tang/src/main/java/org/apache/reef/tang/implementation/TangImpl.java
+++ 
b/lang/java/reef-tang/tang/src/main/java/org/apache/reef/tang/implementation/TangImpl.java
@@ -36,7 +36,7 @@ public class TangImpl implements Tang {
    * it to rebuild them over time.
    */
   public static void reset() {
-    defaultClassHierarchy = new HashMap<>(); //new ClassHierarchyImpl();
+    defaultClassHierarchy = new HashMap<>();
   }
 
   @Override
@@ -51,7 +51,7 @@ public class TangImpl implements Tang {
       return newConfigurationBuilder(new URL[0], new Configuration[0], new 
Class[0]);
     } catch (final BindException e) {
       throw new IllegalStateException(
-          "Caught unexpeceted bind exception!  Implementation bug.", e);
+          "Caught unexpected bind exception! Implementation bug.", e);
     }
   }
 
@@ -67,7 +67,7 @@ public class TangImpl implements Tang {
       return newConfigurationBuilder(jars, new Configuration[0], new Class[0]);
     } catch (final BindException e) {
       throw new IllegalStateException(
-          "Caught unexpeceted bind exception!  Implementation bug.", e);
+          "Caught unexpected bind exception! Implementation bug.", e);
     }
   }
 
@@ -88,11 +88,7 @@ public class TangImpl implements Tang {
   public JavaConfigurationBuilder newConfigurationBuilder(final URL[] jars, 
final Configuration[] confs,
           final Class<? extends ExternalConstructor<?>>[] parameterParsers)
       throws BindException {
-    final JavaConfigurationBuilder cb = new JavaConfigurationBuilderImpl(jars, 
confs, parameterParsers);
-//    for (Configuration c : confs) {
-//      cb.addConfiguration(c);
-//    }
-    return cb;
+    return new JavaConfigurationBuilderImpl(jars, confs, parameterParsers);
   }
 
   @SuppressWarnings("unchecked")
@@ -151,6 +147,12 @@ public class TangImpl implements Tang {
 
     @Override
     public boolean equals(final Object o) {
+      if (this == o) {
+        return true;
+      }
+      if (o == null || getClass() != o.getClass()) {
+        return false;
+      }
       final SetValuedKey other = (SetValuedKey) o;
       if (other.key.size() != this.key.size()) {
         return false;

http://git-wip-us.apache.org/repos/asf/incubator-reef/blob/19809be3/lang/java/reef-tang/tang/src/main/java/org/apache/reef/tang/implementation/types/AbstractNode.java
----------------------------------------------------------------------
diff --git 
a/lang/java/reef-tang/tang/src/main/java/org/apache/reef/tang/implementation/types/AbstractNode.java
 
b/lang/java/reef-tang/tang/src/main/java/org/apache/reef/tang/implementation/types/AbstractNode.java
index 9262bea..70c5394 100644
--- 
a/lang/java/reef-tang/tang/src/main/java/org/apache/reef/tang/implementation/types/AbstractNode.java
+++ 
b/lang/java/reef-tang/tang/src/main/java/org/apache/reef/tang/implementation/types/AbstractNode.java
@@ -55,12 +55,12 @@ public abstract class AbstractNode implements Node {
 
   @Override
   public boolean equals(final Object o) {
-    if (o == null) {
-      return false;
-    }
-    if (o == this) {
+    if (this == o) {
       return true;
     }
+    if (o == null || getClass() != o.getClass()) {
+      return false;
+    }
 
     final AbstractNode n = (AbstractNode) o;
     final boolean parentsEqual;

http://git-wip-us.apache.org/repos/asf/incubator-reef/blob/19809be3/lang/java/reef-tang/tang/src/main/java/org/apache/reef/tang/implementation/types/ConstructorArgImpl.java
----------------------------------------------------------------------
diff --git 
a/lang/java/reef-tang/tang/src/main/java/org/apache/reef/tang/implementation/types/ConstructorArgImpl.java
 
b/lang/java/reef-tang/tang/src/main/java/org/apache/reef/tang/implementation/types/ConstructorArgImpl.java
index 803875f..087e1ed 100644
--- 
a/lang/java/reef-tang/tang/src/main/java/org/apache/reef/tang/implementation/types/ConstructorArgImpl.java
+++ 
b/lang/java/reef-tang/tang/src/main/java/org/apache/reef/tang/implementation/types/ConstructorArgImpl.java
@@ -53,6 +53,12 @@ public class ConstructorArgImpl implements ConstructorArg {
 
   @Override
   public boolean equals(final Object o) {
+    if (this == o) {
+      return true;
+    }
+    if (o == null || getClass() != o.getClass()) {
+      return false;
+    }
     final ConstructorArgImpl arg = (ConstructorArgImpl) o;
     if (!type.equals(arg.type)) {
       return false;

http://git-wip-us.apache.org/repos/asf/incubator-reef/blob/19809be3/lang/java/reef-tang/tang/src/main/java/org/apache/reef/tang/implementation/types/ConstructorDefImpl.java
----------------------------------------------------------------------
diff --git 
a/lang/java/reef-tang/tang/src/main/java/org/apache/reef/tang/implementation/types/ConstructorDefImpl.java
 
b/lang/java/reef-tang/tang/src/main/java/org/apache/reef/tang/implementation/types/ConstructorDefImpl.java
index c2ac2fd..38f290e 100644
--- 
a/lang/java/reef-tang/tang/src/main/java/org/apache/reef/tang/implementation/types/ConstructorDefImpl.java
+++ 
b/lang/java/reef-tang/tang/src/main/java/org/apache/reef/tang/implementation/types/ConstructorDefImpl.java
@@ -120,6 +120,12 @@ public class ConstructorDefImpl<T> implements 
ConstructorDef<T> {
 
   @Override
   public boolean equals(final Object o) {
+    if (this == o) {
+      return true;
+    }
+    if (o == null || getClass() != o.getClass()) {
+      return false;
+    }
     return equalsIgnoreOrder((ConstructorDef<?>) o);
   }
 

http://git-wip-us.apache.org/repos/asf/incubator-reef/blob/19809be3/lang/java/reef-wake/wake/src/main/java/org/apache/reef/wake/remote/impl/SocketRemoteIdentifier.java
----------------------------------------------------------------------
diff --git 
a/lang/java/reef-wake/wake/src/main/java/org/apache/reef/wake/remote/impl/SocketRemoteIdentifier.java
 
b/lang/java/reef-wake/wake/src/main/java/org/apache/reef/wake/remote/impl/SocketRemoteIdentifier.java
index 0914813..a1c1ae0 100644
--- 
a/lang/java/reef-wake/wake/src/main/java/org/apache/reef/wake/remote/impl/SocketRemoteIdentifier.java
+++ 
b/lang/java/reef-wake/wake/src/main/java/org/apache/reef/wake/remote/impl/SocketRemoteIdentifier.java
@@ -90,6 +90,12 @@ public class SocketRemoteIdentifier implements 
RemoteIdentifier {
    */
   @Override
   public boolean equals(final Object o) {
+    if (this == o) {
+      return true;
+    }
+    if (o == null || getClass() != o.getClass()) {
+      return false;
+    }
     return addr.equals(((SocketRemoteIdentifier) o).getSocketAddress());
   }
 

http://git-wip-us.apache.org/repos/asf/incubator-reef/blob/19809be3/lang/java/reef-wake/wake/src/main/java/org/apache/reef/wake/remote/impl/Tuple2.java
----------------------------------------------------------------------
diff --git 
a/lang/java/reef-wake/wake/src/main/java/org/apache/reef/wake/remote/impl/Tuple2.java
 
b/lang/java/reef-wake/wake/src/main/java/org/apache/reef/wake/remote/impl/Tuple2.java
index 13f1d71..ff0ab9d 100644
--- 
a/lang/java/reef-wake/wake/src/main/java/org/apache/reef/wake/remote/impl/Tuple2.java
+++ 
b/lang/java/reef-wake/wake/src/main/java/org/apache/reef/wake/remote/impl/Tuple2.java
@@ -49,6 +49,12 @@ public class Tuple2<T1, T2> {
 
   @Override
   public boolean equals(final Object o) {
+    if (this == o) {
+      return true;
+    }
+    if (o == null || getClass() != o.getClass()) {
+      return false;
+    }
     final Tuple2<T1, T2> tuple = (Tuple2<T1, T2>) o;
     return t1.equals((Object) tuple.getT1()) && t2.equals((Object) 
tuple.getT2());
   }

http://git-wip-us.apache.org/repos/asf/incubator-reef/blob/19809be3/lang/java/reef-wake/wake/src/test/java/org/apache/reef/wake/test/remote/TestRemoteIdentifier.java
----------------------------------------------------------------------
diff --git 
a/lang/java/reef-wake/wake/src/test/java/org/apache/reef/wake/test/remote/TestRemoteIdentifier.java
 
b/lang/java/reef-wake/wake/src/test/java/org/apache/reef/wake/test/remote/TestRemoteIdentifier.java
index bece0df..222128e 100644
--- 
a/lang/java/reef-wake/wake/src/test/java/org/apache/reef/wake/test/remote/TestRemoteIdentifier.java
+++ 
b/lang/java/reef-wake/wake/src/test/java/org/apache/reef/wake/test/remote/TestRemoteIdentifier.java
@@ -32,6 +32,12 @@ public class TestRemoteIdentifier implements 
RemoteIdentifier {
   }
 
   public boolean equals(final Object o) {
+    if (this == o) {
+      return true;
+    }
+    if (o == null || getClass() != o.getClass()) {
+      return false;
+    }
     return str.equals(((TestRemoteIdentifier) o).getString());
   }
 

Reply via email to