Instruction related fixes

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

Change-Id: I178b55f8568c1a9132f0aa88465b8b34dc2b2df2
diff --git a/core/api/src/main/java/org/onosproject/net/flow/instructions/Instructions.java b/core/api/src/main/java/org/onosproject/net/flow/instructions/Instructions.java
index 246d21a..926d267 100644
--- a/core/api/src/main/java/org/onosproject/net/flow/instructions/Instructions.java
+++ b/core/api/src/main/java/org/onosproject/net/flow/instructions/Instructions.java
@@ -260,11 +260,13 @@
         return new TableTypeTransition(type);
     }
 
-    /*
-     *  Drop instructions
+    /**
+     *  Drop instruction.
      */
-
     public static final class DropInstruction implements Instruction {
+
+        private DropInstruction() {}
+
         @Override
         public Type type() {
             return Type.DROP;
@@ -287,18 +289,15 @@
                 return true;
             }
             if (obj instanceof DropInstruction) {
-                DropInstruction that = (DropInstruction) obj;
-                return Objects.equals(type(), that.type());
-
+                return true;
             }
             return false;
         }
     }
 
-    /*
-     *  Output Instruction
+    /**
+     *  Output Instruction.
      */
-
     public static final class OutputInstruction implements Instruction {
         private final PortNumber port;
 
@@ -339,10 +338,9 @@
         }
     }
 
-    /*
-     *  Group Instruction
+    /**
+     *  Group Instruction.
      */
-
     public static final class GroupInstruction implements Instruction {
         private final GroupId groupId;
 
@@ -389,7 +387,7 @@
     public static class TableTypeTransition implements Instruction {
         private final FlowRule.Type tableType;
 
-        public TableTypeTransition(FlowRule.Type type) {
+        TableTypeTransition(FlowRule.Type type) {
             this.tableType = type;
         }
 
@@ -405,7 +403,7 @@
         @Override
         public String toString() {
             return toStringHelper(type().toString())
-                    .add("group ID", this.tableType).toString();
+                    .add("tableType", this.tableType).toString();
         }
 
         @Override
diff --git a/core/api/src/main/java/org/onosproject/net/flow/instructions/L0ModificationInstruction.java b/core/api/src/main/java/org/onosproject/net/flow/instructions/L0ModificationInstruction.java
index 69d6938..ba5b7d7 100644
--- a/core/api/src/main/java/org/onosproject/net/flow/instructions/L0ModificationInstruction.java
+++ b/core/api/src/main/java/org/onosproject/net/flow/instructions/L0ModificationInstruction.java
@@ -36,7 +36,7 @@
     public abstract L0SubType subtype();
 
     @Override
-    public Type type() {
+    public final Type type() {
         return Type.L0MODIFICATION;
     }
 
@@ -48,7 +48,7 @@
         private final L0SubType subtype;
         private final short lambda;
 
-        public ModLambdaInstruction(L0SubType subType, short lambda) {
+        ModLambdaInstruction(L0SubType subType, short lambda) {
             this.subtype = subType;
             this.lambda = lambda;
         }
@@ -81,7 +81,6 @@
             if (obj instanceof ModLambdaInstruction) {
                 ModLambdaInstruction that = (ModLambdaInstruction) obj;
                 return  Objects.equals(lambda, that.lambda) &&
-                        Objects.equals(this.type(), that.type()) &&
                         Objects.equals(subtype, that.subtype);
             }
             return false;
diff --git a/core/api/src/main/java/org/onosproject/net/flow/instructions/L2ModificationInstruction.java b/core/api/src/main/java/org/onosproject/net/flow/instructions/L2ModificationInstruction.java
index 5135557..2288898 100644
--- a/core/api/src/main/java/org/onosproject/net/flow/instructions/L2ModificationInstruction.java
+++ b/core/api/src/main/java/org/onosproject/net/flow/instructions/L2ModificationInstruction.java
@@ -80,7 +80,7 @@
     public abstract L2SubType subtype();
 
     @Override
-    public Type type() {
+    public final Type type() {
         return Type.L2MODIFICATION;
     }
 
@@ -92,7 +92,7 @@
         private final L2SubType subtype;
         private final MacAddress mac;
 
-        public ModEtherInstruction(L2SubType subType, MacAddress addr) {
+        ModEtherInstruction(L2SubType subType, MacAddress addr) {
 
             this.subtype = subType;
             this.mac = addr;
@@ -126,16 +126,13 @@
             if (obj instanceof ModEtherInstruction) {
                 ModEtherInstruction that = (ModEtherInstruction) obj;
                 return  Objects.equals(mac, that.mac) &&
-                        Objects.equals(this.type(), that.type()) &&
                         Objects.equals(subtype, that.subtype);
-
             }
             return false;
         }
-
-
     }
 
+    // TODO This instruction is reused for Pop-Mpls. Consider renaming.
     public static final class PushHeaderInstructions extends
             L2ModificationInstruction {
 
@@ -191,7 +188,7 @@
 
         private final VlanId vlanId;
 
-        public ModVlanIdInstruction(VlanId vlanId) {
+        ModVlanIdInstruction(VlanId vlanId) {
             this.vlanId = vlanId;
         }
 
@@ -222,15 +219,10 @@
             }
             if (obj instanceof ModVlanIdInstruction) {
                 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;
         }
-
-
     }
 
     /**
@@ -240,7 +232,7 @@
 
         private final Byte vlanPcp;
 
-        public ModVlanPcpInstruction(Byte vlanPcp) {
+        ModVlanPcpInstruction(Byte vlanPcp) {
             this.vlanPcp = vlanPcp;
         }
 
@@ -271,26 +263,22 @@
             }
             if (obj instanceof ModVlanPcpInstruction) {
                 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;
         }
-
     }
 
 
     /**
      * Represents a MPLS label modification.
      */
-    public static final class ModMplsLabelInstruction extends
-    L2ModificationInstruction {
+    public static final class ModMplsLabelInstruction
+            extends L2ModificationInstruction {
 
         private final MplsLabel mplsLabel;
 
-        public ModMplsLabelInstruction(MplsLabel mplsLabel) {
+        ModMplsLabelInstruction(MplsLabel mplsLabel) {
             this.mplsLabel = mplsLabel;
         }
 
@@ -321,10 +309,7 @@
             }
             if (obj instanceof ModMplsLabelInstruction) {
                 ModMplsLabelInstruction that = (ModMplsLabelInstruction) obj;
-                return Objects.equals(mplsLabel, that.mplsLabel) &&
-                        Objects.equals(this.type(), that.type());
-
-
+                return Objects.equals(mplsLabel, that.mplsLabel);
             }
             return false;
         }
@@ -333,10 +318,10 @@
     /**
      * Represents a MPLS TTL modification.
      */
-    public static final class ModMplsTtlInstruction extends
-            L2ModificationInstruction {
+    public static final class ModMplsTtlInstruction
+            extends L2ModificationInstruction {
 
-        public ModMplsTtlInstruction() {
+        ModMplsTtlInstruction() {
         }
 
         @Override
diff --git a/core/api/src/main/java/org/onosproject/net/flow/instructions/L3ModificationInstruction.java b/core/api/src/main/java/org/onosproject/net/flow/instructions/L3ModificationInstruction.java
index 88a26b9..3d0944e 100644
--- a/core/api/src/main/java/org/onosproject/net/flow/instructions/L3ModificationInstruction.java
+++ b/core/api/src/main/java/org/onosproject/net/flow/instructions/L3ModificationInstruction.java
@@ -80,7 +80,7 @@
     public abstract L3SubType subtype();
 
     @Override
-    public Type type() {
+    public final Type type() {
         return Type.L3MODIFICATION;
     }
 
@@ -92,7 +92,7 @@
         private final L3SubType subtype;
         private final IpAddress ip;
 
-        public ModIPInstruction(L3SubType subType, IpAddress addr) {
+        ModIPInstruction(L3SubType subType, IpAddress addr) {
 
             this.subtype = subType;
             this.ip = addr;
@@ -126,9 +126,7 @@
             if (obj instanceof ModIPInstruction) {
                 ModIPInstruction that = (ModIPInstruction) obj;
                 return  Objects.equals(ip, that.ip) &&
-                        Objects.equals(this.type(), that.type()) &&
                         Objects.equals(this.subtype(), that.subtype());
-
             }
             return false;
         }
@@ -148,7 +146,7 @@
          *
          * @param flowLabel the IPv6 flow label to set in the treatment (20 bits)
          */
-        public ModIPv6FlowLabelInstruction(int flowLabel) {
+        ModIPv6FlowLabelInstruction(int flowLabel) {
             this.flowLabel = flowLabel & MASK;
         }
 
@@ -185,9 +183,7 @@
             if (obj instanceof ModIPv6FlowLabelInstruction) {
                 ModIPv6FlowLabelInstruction that =
                     (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;
         }
@@ -200,7 +196,7 @@
 
         private final L3SubType subtype;
 
-        public ModTtlInstruction(L3SubType subtype) {
+        ModTtlInstruction(L3SubType subtype) {
             this.subtype = subtype;
         }
 
@@ -227,9 +223,7 @@
             }
             if (obj instanceof ModTtlInstruction) {
                 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;
         }
diff --git a/web/api/src/test/java/org/onosproject/codec/impl/InstructionCodecTest.java b/web/api/src/test/java/org/onosproject/codec/impl/InstructionCodecTest.java
index 78685c1..949707b 100644
--- a/web/api/src/test/java/org/onosproject/codec/impl/InstructionCodecTest.java
+++ b/web/api/src/test/java/org/onosproject/codec/impl/InstructionCodecTest.java
@@ -73,7 +73,7 @@
     @Test
     public void dropInstructionTest() {
         final Instructions.DropInstruction instruction =
-                new Instructions.DropInstruction();
+                Instructions.createDrop();
         final ObjectNode instructionJson =
                 instructionCodec.encode(instruction, context);
         assertThat(instructionJson, matchesInstruction(instruction));