HADOOP-4210. Fix findbugs warnings for equals implementations of mapred ID

classes. Removed public, static ID::read and ID::forName; made ID an
abstract class. Contributed by Suresh Srinivas.


git-svn-id: https://svn.apache.org/repos/asf/hadoop/core/trunk@700548 13f79535-47bb-0310-9956-ffa450edef68

CHANGES.txt

@@ -4,6 +4,10 @@ Trunk (unreleased changes)
 
   INCOMPATIBLE CHANGES
 
+    HADOOP-4210. Fix findbugs warnings for equals implementations of mapred ID
+    classes. Removed public, static ID::read and ID::forName; made ID an
+    abstract class. (Suresh Srinivas via cdouglas)
+
   NEW FEATURES
 
   IMPROVEMENTS

src/core/org/apache/hadoop/record/meta/MapTypeID.java

@@ -62,14 +62,13 @@ public class MapTypeID extends TypeID {
    * same type
    */
   public boolean equals(Object o) {
-    if (this == o) 
-      return true;
-    if (!(o instanceof MapTypeID))
+    if (!super.equals(o))
       return false;
+
     MapTypeID mti = (MapTypeID) o;
-    if (!this.typeIDKey.equals(mti.typeIDKey))
-      return false;
-    return this.typeIDValue.equals(mti.typeIDValue);
+
+    return this.typeIDKey.equals(mti.typeIDKey) &&
+           this.typeIDValue.equals(mti.typeIDValue);
   }
   
   /**

src/core/org/apache/hadoop/record/meta/TypeID.java

@@ -84,8 +84,13 @@ public class TypeID {
   public boolean equals(Object o) {
     if (this == o) 
       return true;
-    if (!(o instanceof TypeID))
+
+    if (o == null)
+      return false;
+
+    if (this.getClass() != o.getClass())
       return false;
+
     TypeID oTypeID = (TypeID) o;
     return (this.typeVal == oTypeID.typeVal);
   }

src/core/org/apache/hadoop/record/meta/VectorTypeID.java

@@ -47,10 +47,9 @@ public class VectorTypeID extends TypeID {
    * same type
    */
   public boolean equals(Object o) {
-    if (this == o) 
-      return true;
-    if (!(o instanceof VectorTypeID))
+    if (!super.equals (o))
       return false;
+
     VectorTypeID vti = (VectorTypeID) o;
     return this.typeIDElement.equals(vti.typeIDElement);
   }

src/mapred/org/apache/hadoop/mapred/ID.java

@@ -33,7 +33,7 @@ import org.apache.hadoop.io.WritableComparable;
  * @see TaskID
  * @see TaskAttemptID
  */
-public class ID implements WritableComparable<ID> {
+public abstract class ID implements WritableComparable<ID> {
   protected int id;
 
   /** constructs an ID object from the given int */
@@ -61,9 +61,11 @@ public class ID implements WritableComparable<ID> {
 
   @Override
   public boolean equals(Object o) {
+    if (this == o)
+      return true;
     if(o == null)
       return false;
-    if (o.getClass().equals(ID.class)) {
+    if (o.getClass() == this.getClass()) {
       ID that = (ID) o;
       return this.id == that.id;
     }
@@ -83,29 +85,4 @@ public class ID implements WritableComparable<ID> {
   public void write(DataOutput out) throws IOException {
     out.writeInt(id);
   }
-
-  public static ID read(DataInput in) throws IOException {
-    ID id = new ID();
-    id.readFields(in);
-    return id;
-  }
-
-  /**
-   * Construct an ID object from given string
-   * 
-   * @return constructed Id object or null if the given String is null
-   * @throws IllegalArgumentException if the given string is malformed
-   */
-  public static ID forName(String str) throws IllegalArgumentException {
-    if (str == null)
-      return null;
-    try {
-      int id = Integer.parseInt(str);
-      return new ID(id);
-    }
-    catch (Exception ex) {
-      throw new IllegalArgumentException("Id string : " + str
-          + " is not propoerly formed");
-    }
-  }
 }

src/mapred/org/apache/hadoop/mapred/JobID.java

@@ -73,14 +73,11 @@ public class JobID extends ID {
   
   @Override
   public boolean equals(Object o) {
-    if(o == null)
+    if (!super.equals(o))
       return false;
-    if(o.getClass().equals(JobID.class)) {
-      JobID that = (JobID)o;
-      return this.id==that.id
-        && this.jtIdentifier.equals(that.jtIdentifier);
-    }
-    else return false;
+
+    JobID that = (JobID)o;
+    return this.jtIdentifier.equals(that.jtIdentifier);
   }
   
   /**Compare JobIds by first jtIdentifiers, then by job numbers*/

src/mapred/org/apache/hadoop/mapred/TaskAttemptID.java

@@ -125,15 +125,12 @@ public class TaskAttemptID extends ID {
   
   @Override
   public boolean equals(Object o) {
-    if(o == null)
+    if (!super.equals(o))
       return false;
-    if(o.getClass().equals(TaskAttemptID.class)) {
-      TaskAttemptID that = (TaskAttemptID)o;
-      return this.id==that.id
-             && this.taskId.equals(that.taskId) 
-             && this.jtTimestamp == that.jtTimestamp;
-    }
-    else return false;
+
+    TaskAttemptID that = (TaskAttemptID)o;
+    return this.taskId.equals(that.taskId) && 
+           this.jtTimestamp == that.jtTimestamp;
   }
   
   /**Compare TaskIds by first tipIds, then by task numbers. */

src/mapred/org/apache/hadoop/mapred/TaskID.java

@@ -97,15 +97,11 @@ public class TaskID extends ID {
   
   @Override
   public boolean equals(Object o) {
-    if(o == null)
+    if (!super.equals(o))
       return false;
-    if(o.getClass().equals(TaskID.class)) {
-      TaskID that = (TaskID)o;
-      return this.id==that.id
-        && this.isMap == that.isMap
-        && this.jobId.equals(that.jobId);
-    }
-    else return false;
+
+    TaskID that = (TaskID)o;
+    return this.isMap == that.isMap && this.jobId.equals(that.jobId);
   }
 
   /**Compare TaskInProgressIds by first jobIds, then by tip numbers. Reduces are 

src/mapred/org/apache/hadoop/mapreduce/ID.java

@@ -33,7 +33,7 @@ import org.apache.hadoop.io.WritableComparable;
  * @see TaskID
  * @see TaskAttemptID
  */
-public class ID implements WritableComparable<ID> {
+public abstract class ID implements WritableComparable<ID> {
   protected int id;
 
   /** constructs an ID object from the given int */
@@ -61,9 +61,11 @@ public class ID implements WritableComparable<ID> {
 
   @Override
   public boolean equals(Object o) {
+    if (this == o)
+      return true;
     if(o == null)
       return false;
-    if (o.getClass().equals(ID.class)) {
+    if (o.getClass() == this.getClass()) {
       ID that = (ID) o;
       return this.id == that.id;
     }
@@ -83,29 +85,4 @@ public class ID implements WritableComparable<ID> {
   public void write(DataOutput out) throws IOException {
     out.writeInt(id);
   }
-
-  public static ID read(DataInput in) throws IOException {
-    ID id = new ID();
-    id.readFields(in);
-    return id;
-  }
-
-  /**
-   * Construct an ID object from given string
-   * 
-   * @return constructed Id object or null if the given String is null
-   * @throws IllegalArgumentException if the given string is malformed
-   */
-  public static ID forName(String str) throws IllegalArgumentException {
-    if (str == null)
-      return null;
-    try {
-      int id = Integer.parseInt(str);
-      return new ID(id);
-    }
-    catch (Exception ex) {
-      throw new IllegalArgumentException("Id string : " + str
-          + " is not propoerly formed");
-    }
-  }
 }

src/mapred/org/apache/hadoop/mapreduce/JobID.java

@@ -74,14 +74,11 @@ public class JobID extends ID {
   
   @Override
   public boolean equals(Object o) {
-    if(o == null)
+    if (!super.equals(o))
       return false;
-    if(o.getClass().equals(JobID.class)) {
-      JobID that = (JobID)o;
-      return this.id==that.id
-        && this.jtIdentifier.equals(that.jtIdentifier);
-    }
-    else return false;
+
+    JobID that = (JobID)o;
+    return this.jtIdentifier.equals(that.jtIdentifier);
   }
   
   /**Compare JobIds by first jtIdentifiers, then by job numbers*/

src/mapred/org/apache/hadoop/mapreduce/TaskAttemptID.java

@@ -92,14 +92,11 @@ public class TaskAttemptID extends ID {
   
   @Override
   public boolean equals(Object o) {
-    if(o == null)
+    if (!super.equals(o))
       return false;
-    if(o.getClass().equals(TaskAttemptID.class)) {
-      TaskAttemptID that = (TaskAttemptID)o;
-      return this.id==that.id
-        && this.taskId.equals(that.taskId);
-    }
-    else return false;
+
+    TaskAttemptID that = (TaskAttemptID)o;
+    return this.taskId.equals(that.taskId);
   }
   
   /**Compare TaskIds by first tipIds, then by task numbers. */

src/mapred/org/apache/hadoop/mapreduce/TaskID.java

@@ -97,15 +97,11 @@ public class TaskID extends ID {
   
   @Override
   public boolean equals(Object o) {
-    if(o == null)
+    if (!super.equals(o))
       return false;
-    if(o.getClass().equals(TaskID.class)) {
-      TaskID that = (TaskID)o;
-      return this.id==that.id
-        && this.isMap == that.isMap
-        && this.jobId.equals(that.jobId);
-    }
-    else return false;
+
+    TaskID that = (TaskID)o;
+    return this.isMap == that.isMap && this.jobId.equals(that.jobId);
   }
 
   /**Compare TaskInProgressIds by first jobIds, then by tip numbers. Reduces are