Instruction related fixes

- Removed redundant equality check. (ONOS-975)
- Enforced using Instruction Factory methods.
- cosmetic fixes.

Change-Id: I178b55f8568c1a9132f0aa88465b8b34dc2b2df2

@@ -260,11 +260,13 @@ public final class Instructions {
         return new TableTypeTransition(type);
         return new TableTypeTransition(type);
     }
     }
 
 
-    /*
-     *  Drop instructions
+    /**
+     *  Drop instruction.
      */
      */
-
     public static final class DropInstruction implements Instruction {
     public static final class DropInstruction implements Instruction {
+
+        private DropInstruction() {}
+
         @Override
         @Override
         public Type type() {
         public Type type() {
             return Type.DROP;
             return Type.DROP;
@@ -287,18 +289,15 @@ public final class Instructions {
                 return true;
                 return true;
             }
             }
             if (obj instanceof DropInstruction) {
             if (obj instanceof DropInstruction) {
-                DropInstruction that = (DropInstruction) obj;
-                return Objects.equals(type(), that.type());
-
+                return true;
             }
             }
             return false;
             return false;
         }
         }
     }
     }
 
 
-    /*
-     *  Output Instruction
+    /**
+     *  Output Instruction.
      */
      */
-
     public static final class OutputInstruction implements Instruction {
     public static final class OutputInstruction implements Instruction {
         private final PortNumber port;
         private final PortNumber port;
 
 
@@ -339,10 +338,9 @@ public final class Instructions {
         }
         }
     }
     }
 
 
-    /*
-     *  Group Instruction
+    /**
+     *  Group Instruction.
      */
      */
-
     public static final class GroupInstruction implements Instruction {
     public static final class GroupInstruction implements Instruction {
         private final GroupId groupId;
         private final GroupId groupId;
 
 
@@ -389,7 +387,7 @@ public final class Instructions {
     public static class TableTypeTransition implements Instruction {
     public static class TableTypeTransition implements Instruction {
         private final FlowRule.Type tableType;
         private final FlowRule.Type tableType;
 
 
-        public TableTypeTransition(FlowRule.Type type) {
+        TableTypeTransition(FlowRule.Type type) {
             this.tableType = type;
             this.tableType = type;
         }
         }
 
 
@@ -405,7 +403,7 @@ public final class Instructions {
         @Override
         @Override
         public String toString() {
         public String toString() {
             return toStringHelper(type().toString())
             return toStringHelper(type().toString())
-                    .add("group ID", this.tableType).toString();
+                    .add("tableType", this.tableType).toString();
         }
         }
 
 
         @Override
         @Override

@@ -36,7 +36,7 @@ public abstract class L0ModificationInstruction implements Instruction {
     public abstract L0SubType subtype();
     public abstract L0SubType subtype();
 
 
     @Override
     @Override
-    public Type type() {
+    public final Type type() {
         return Type.L0MODIFICATION;
         return Type.L0MODIFICATION;
     }
     }
 
 
@@ -48,7 +48,7 @@ public abstract class L0ModificationInstruction implements Instruction {
         private final L0SubType subtype;
         private final L0SubType subtype;
         private final short lambda;
         private final short lambda;
 
 
-        public ModLambdaInstruction(L0SubType subType, short lambda) {
+        ModLambdaInstruction(L0SubType subType, short lambda) {
             this.subtype = subType;
             this.subtype = subType;
             this.lambda = lambda;
             this.lambda = lambda;
         }
         }
@@ -81,7 +81,6 @@ public abstract class L0ModificationInstruction implements Instruction {
             if (obj instanceof ModLambdaInstruction) {
             if (obj instanceof ModLambdaInstruction) {
                 ModLambdaInstruction that = (ModLambdaInstruction) obj;
                 ModLambdaInstruction that = (ModLambdaInstruction) obj;
                 return  Objects.equals(lambda, that.lambda) &&
                 return  Objects.equals(lambda, that.lambda) &&
-                        Objects.equals(this.type(), that.type()) &&
                         Objects.equals(subtype, that.subtype);
                         Objects.equals(subtype, that.subtype);
             }
             }
             return false;
             return false;

@@ -80,7 +80,7 @@ public abstract class L2ModificationInstruction implements Instruction {
     public abstract L2SubType subtype();
     public abstract L2SubType subtype();
 
 
     @Override
     @Override
-    public Type type() {
+    public final Type type() {
         return Type.L2MODIFICATION;
         return Type.L2MODIFICATION;
     }
     }
 
 
@@ -92,7 +92,7 @@ public abstract class L2ModificationInstruction implements Instruction {
         private final L2SubType subtype;
         private final L2SubType subtype;
         private final MacAddress mac;
         private final MacAddress mac;
 
 
-        public ModEtherInstruction(L2SubType subType, MacAddress addr) {
+        ModEtherInstruction(L2SubType subType, MacAddress addr) {
 
 
             this.subtype = subType;
             this.subtype = subType;
             this.mac = addr;
             this.mac = addr;
@@ -126,16 +126,13 @@ public abstract class L2ModificationInstruction implements Instruction {
             if (obj instanceof ModEtherInstruction) {
             if (obj instanceof ModEtherInstruction) {
                 ModEtherInstruction that = (ModEtherInstruction) obj;
                 ModEtherInstruction that = (ModEtherInstruction) obj;
                 return  Objects.equals(mac, that.mac) &&
                 return  Objects.equals(mac, that.mac) &&
-                        Objects.equals(this.type(), that.type()) &&
                         Objects.equals(subtype, that.subtype);
                         Objects.equals(subtype, that.subtype);
-
             }
             }
             return false;
             return false;
         }
         }
-
-
     }
     }
 
 
+    // TODO This instruction is reused for Pop-Mpls. Consider renaming.
     public static final class PushHeaderInstructions extends
     public static final class PushHeaderInstructions extends
             L2ModificationInstruction {
             L2ModificationInstruction {
 
 
@@ -191,7 +188,7 @@ public abstract class L2ModificationInstruction implements Instruction {
 
 
         private final VlanId vlanId;
         private final VlanId vlanId;
 
 
-        public ModVlanIdInstruction(VlanId vlanId) {
+        ModVlanIdInstruction(VlanId vlanId) {
             this.vlanId = vlanId;
             this.vlanId = vlanId;
         }
         }
 
 
@@ -222,15 +219,10 @@ public abstract class L2ModificationInstruction implements Instruction {
             }
             }
             if (obj instanceof ModVlanIdInstruction) {
             if (obj instanceof ModVlanIdInstruction) {
                 ModVlanIdInstruction that = (ModVlanIdInstruction) obj;
                 ModVlanIdInstruction that = (ModVlanIdInstruction) obj;
-                return  Objects.equals(vlanId, that.vlanId) &&
-                        Objects.equals(this.type(), that.type()) &&
-                        Objects.equals(this.subtype(), that.subtype());
-
+                return  Objects.equals(vlanId, that.vlanId);
             }
             }
             return false;
             return false;
         }
         }
-
-
     }
     }
 
 
     /**
     /**
@@ -240,7 +232,7 @@ public abstract class L2ModificationInstruction implements Instruction {
 
 
         private final Byte vlanPcp;
         private final Byte vlanPcp;
 
 
-        public ModVlanPcpInstruction(Byte vlanPcp) {
+        ModVlanPcpInstruction(Byte vlanPcp) {
             this.vlanPcp = vlanPcp;
             this.vlanPcp = vlanPcp;
         }
         }
 
 
@@ -271,26 +263,22 @@ public abstract class L2ModificationInstruction implements Instruction {
             }
             }
             if (obj instanceof ModVlanPcpInstruction) {
             if (obj instanceof ModVlanPcpInstruction) {
                 ModVlanPcpInstruction that = (ModVlanPcpInstruction) obj;
                 ModVlanPcpInstruction that = (ModVlanPcpInstruction) obj;
-                return  Objects.equals(vlanPcp, that.vlanPcp) &&
-                        Objects.equals(this.type(), that.type()) &&
-                        Objects.equals(this.subtype(), that.subtype());
-
+                return  Objects.equals(vlanPcp, that.vlanPcp);
             }
             }
             return false;
             return false;
         }
         }
-
     }
     }
 
 
 
 
     /**
     /**
      * Represents a MPLS label modification.
      * Represents a MPLS label modification.
      */
      */
-    public static final class ModMplsLabelInstruction extends
-    L2ModificationInstruction {
+    public static final class ModMplsLabelInstruction
+            extends L2ModificationInstruction {
 
 
         private final MplsLabel mplsLabel;
         private final MplsLabel mplsLabel;
 
 
-        public ModMplsLabelInstruction(MplsLabel mplsLabel) {
+        ModMplsLabelInstruction(MplsLabel mplsLabel) {
             this.mplsLabel = mplsLabel;
             this.mplsLabel = mplsLabel;
         }
         }
 
 
@@ -321,10 +309,7 @@ public abstract class L2ModificationInstruction implements Instruction {
             }
             }
             if (obj instanceof ModMplsLabelInstruction) {
             if (obj instanceof ModMplsLabelInstruction) {
                 ModMplsLabelInstruction that = (ModMplsLabelInstruction) obj;
                 ModMplsLabelInstruction that = (ModMplsLabelInstruction) obj;
-                return Objects.equals(mplsLabel, that.mplsLabel) &&
-                        Objects.equals(this.type(), that.type());
-
-
+                return Objects.equals(mplsLabel, that.mplsLabel);
             }
             }
             return false;
             return false;
         }
         }
@@ -333,10 +318,10 @@ public abstract class L2ModificationInstruction implements Instruction {
     /**
     /**
      * Represents a MPLS TTL modification.
      * Represents a MPLS TTL modification.
      */
      */
-    public static final class ModMplsTtlInstruction extends
-            L2ModificationInstruction {
+    public static final class ModMplsTtlInstruction
+            extends L2ModificationInstruction {
 
 
-        public ModMplsTtlInstruction() {
+        ModMplsTtlInstruction() {
         }
         }
 
 
         @Override
         @Override

@@ -80,7 +80,7 @@ public abstract class L3ModificationInstruction implements Instruction {
     public abstract L3SubType subtype();
     public abstract L3SubType subtype();
 
 
     @Override
     @Override
-    public Type type() {
+    public final Type type() {
         return Type.L3MODIFICATION;
         return Type.L3MODIFICATION;
     }
     }
 
 
@@ -92,7 +92,7 @@ public abstract class L3ModificationInstruction implements Instruction {
         private final L3SubType subtype;
         private final L3SubType subtype;
         private final IpAddress ip;
         private final IpAddress ip;
 
 
-        public ModIPInstruction(L3SubType subType, IpAddress addr) {
+        ModIPInstruction(L3SubType subType, IpAddress addr) {
 
 
             this.subtype = subType;
             this.subtype = subType;
             this.ip = addr;
             this.ip = addr;
@@ -126,9 +126,7 @@ public abstract class L3ModificationInstruction implements Instruction {
             if (obj instanceof ModIPInstruction) {
             if (obj instanceof ModIPInstruction) {
                 ModIPInstruction that = (ModIPInstruction) obj;
                 ModIPInstruction that = (ModIPInstruction) obj;
                 return  Objects.equals(ip, that.ip) &&
                 return  Objects.equals(ip, that.ip) &&
-                        Objects.equals(this.type(), that.type()) &&
                         Objects.equals(this.subtype(), that.subtype());
                         Objects.equals(this.subtype(), that.subtype());
-
             }
             }
             return false;
             return false;
         }
         }
@@ -148,7 +146,7 @@ public abstract class L3ModificationInstruction implements Instruction {
          *
          *
          * @param flowLabel the IPv6 flow label to set in the treatment (20 bits)
          * @param flowLabel the IPv6 flow label to set in the treatment (20 bits)
          */
          */
-        public ModIPv6FlowLabelInstruction(int flowLabel) {
+        ModIPv6FlowLabelInstruction(int flowLabel) {
             this.flowLabel = flowLabel & MASK;
             this.flowLabel = flowLabel & MASK;
         }
         }
 
 
@@ -185,9 +183,7 @@ public abstract class L3ModificationInstruction implements Instruction {
             if (obj instanceof ModIPv6FlowLabelInstruction) {
             if (obj instanceof ModIPv6FlowLabelInstruction) {
                 ModIPv6FlowLabelInstruction that =
                 ModIPv6FlowLabelInstruction that =
                     (ModIPv6FlowLabelInstruction) obj;
                     (ModIPv6FlowLabelInstruction) obj;
-                return  Objects.equals(flowLabel, that.flowLabel) &&
-                        Objects.equals(this.type(), that.type()) &&
-                        Objects.equals(this.subtype(), that.subtype());
+                return  Objects.equals(flowLabel, that.flowLabel);
             }
             }
             return false;
             return false;
         }
         }
@@ -200,7 +196,7 @@ public abstract class L3ModificationInstruction implements Instruction {
 
 
         private final L3SubType subtype;
         private final L3SubType subtype;
 
 
-        public ModTtlInstruction(L3SubType subtype) {
+        ModTtlInstruction(L3SubType subtype) {
             this.subtype = subtype;
             this.subtype = subtype;
         }
         }
 
 
@@ -227,9 +223,7 @@ public abstract class L3ModificationInstruction implements Instruction {
             }
             }
             if (obj instanceof ModTtlInstruction) {
             if (obj instanceof ModTtlInstruction) {
                 ModTtlInstruction that = (ModTtlInstruction) obj;
                 ModTtlInstruction that = (ModTtlInstruction) obj;
-                return  Objects.equals(this.subtype(), that.subtype()) &&
-                        Objects.equals(this.type(), that.type());
-
+                return  Objects.equals(this.subtype(), that.subtype());
             }
             }
             return false;
             return false;
         }
         }

@@ -73,7 +73,7 @@ public class InstructionCodecTest {
     @Test
     @Test
     public void dropInstructionTest() {
     public void dropInstructionTest() {
         final Instructions.DropInstruction instruction =
         final Instructions.DropInstruction instruction =
-                new Instructions.DropInstruction();
+                Instructions.createDrop();
         final ObjectNode instructionJson =
         final ObjectNode instructionJson =
                 instructionCodec.encode(instruction, context);
                 instructionCodec.encode(instruction, context);
         assertThat(instructionJson, matchesInstruction(instruction));
         assertThat(instructionJson, matchesInstruction(instruction));