From 398491c5f9629ead21e1d1bf975c8372a84c6ebf Mon Sep 17 00:00:00 2001
From: gmungoc <g.m.carstairs@dundee.ac.uk>
Date: Wed, 24 May 2017 16:42:50 +0100
Subject: [PATCH] JAL-2505 make SequenceFeature type, begin, end, featureGroup
 final

---
 src/jalview/datamodel/SequenceFeature.java |  102 ++++++++++++----------------
 test/jalview/datamodel/SequenceTest.java   |    5 +-
 2 files changed, 47 insertions(+), 60 deletions(-)

diff --git a/src/jalview/datamodel/SequenceFeature.java b/src/jalview/datamodel/SequenceFeature.java
index fa29ffd..627e1ae 100755
--- a/src/jalview/datamodel/SequenceFeature.java
+++ b/src/jalview/datamodel/SequenceFeature.java
@@ -50,13 +50,20 @@ public class SequenceFeature implements FeatureLocationI
    */
   private static final String ATTRIBUTES = "ATTRIBUTES";
 
-  public int begin;
+  /*
+   * type, begin, end, featureGroup are final to ensure that
+   * the integrity of SequenceFeatures data store can't be
+   * broken by direct update of these fields
+   */
+  public final String type;
 
-  public int end;
+  public final int begin;
 
-  public float score;
+  public final int end;
+
+  public final String featureGroup;
 
-  public String type;
+  public float score;
 
   public String description;
 
@@ -68,14 +75,6 @@ public class SequenceFeature implements FeatureLocationI
 
   public Vector<String> links;
 
-  // Feature group can be set from a features file
-  // as a group of features between STARTGROUP and ENDGROUP markers
-  public String featureGroup;
-
-  public SequenceFeature()
-  {
-  }
-
   /**
    * Constructs a duplicate feature. Note: Uses makes a shallow copy of the
    * otherDetails map, so the new and original SequenceFeature may reference the
@@ -85,41 +84,27 @@ public class SequenceFeature implements FeatureLocationI
    */
   public SequenceFeature(SequenceFeature cpy)
   {
-    if (cpy != null)
+    this(cpy, cpy.getBegin(), cpy.getEnd(), cpy.getFeatureGroup());
+
+    score = cpy.score;
+    description = cpy.description;
+    if (cpy.otherDetails != null)
     {
-      begin = cpy.begin;
-      end = cpy.end;
-      score = cpy.score;
-      if (cpy.type != null)
-      {
-        type = new String(cpy.type);
-      }
-      if (cpy.description != null)
+      try
       {
-        description = new String(cpy.description);
-      }
-      if (cpy.featureGroup != null)
-      {
-        featureGroup = new String(cpy.featureGroup);
-      }
-      if (cpy.otherDetails != null)
+        otherDetails = (Map<String, Object>) ((HashMap<String, Object>) cpy.otherDetails)
+                .clone();
+      } catch (Exception e)
       {
-        try
-        {
-          otherDetails = (Map<String, Object>) ((HashMap<String, Object>) cpy.otherDetails)
-                  .clone();
-        } catch (Exception e)
-        {
-          // ignore
-        }
+        // ignore
       }
-      if (cpy.links != null && cpy.links.size() > 0)
+    }
+    if (cpy.links != null && cpy.links.size() > 0)
+    {
+      links = new Vector<String>();
+      for (int i = 0, iSize = cpy.links.size(); i < iSize; i++)
       {
-        links = new Vector<String>();
-        for (int i = 0, iSize = cpy.links.size(); i < iSize; i++)
-        {
-          links.addElement(cpy.links.elementAt(i));
-        }
+        links.addElement(cpy.links.elementAt(i));
       }
     }
   }
@@ -189,10 +174,24 @@ public class SequenceFeature implements FeatureLocationI
   public SequenceFeature(SequenceFeature sf, int newBegin, int newEnd,
           String newGroup)
   {
-    this(sf);
-    begin = newBegin;
-    end = newEnd;
-    featureGroup = newGroup;
+    this(sf.getType(), newBegin, newEnd, newGroup);
+  }
+
+  /**
+   * Constructor that sets the final fields type, begin, end, group
+   * 
+   * @param theType
+   * @param theBegin
+   * @param theEnd
+   * @param theGroup
+   */
+  private SequenceFeature(String theType, int theBegin, int theEnd,
+          String theGroup)
+  {
+    type = theType;
+    begin = theBegin;
+    end = theEnd;
+    featureGroup = theGroup;
   }
 
   /**
@@ -437,17 +436,6 @@ public class SequenceFeature implements FeatureLocationI
     return (String) getValue(ATTRIBUTES);
   }
 
-  public void setPosition(int pos)
-  {
-    begin = pos;
-    end = pos;
-  }
-
-  public int getPosition()
-  {
-    return begin;
-  }
-
   /**
    * Return 1 for forward strand ('+' in GFF), -1 for reverse strand ('-' in
    * GFF), and 0 for unknown or not (validly) specified
diff --git a/test/jalview/datamodel/SequenceTest.java b/test/jalview/datamodel/SequenceTest.java
index ebf4857..e3e39f7 100644
--- a/test/jalview/datamodel/SequenceTest.java
+++ b/test/jalview/datamodel/SequenceTest.java
@@ -41,8 +41,6 @@ import java.util.Vector;
 
 import junit.extensions.PA;
 
-import junit.extensions.PA;
-
 import org.testng.Assert;
 import org.testng.annotations.BeforeClass;
 import org.testng.annotations.BeforeMethod;
@@ -468,7 +466,8 @@ public class SequenceTest
      * Note JAL-2046: spurious: we have no use case for this at the moment.
      * This test also buggy - as sf2.equals(sf), no new feature is added
      */
-    SequenceFeature sf2 = new SequenceFeature();
+    SequenceFeature sf2 = new SequenceFeature("Scop", "desc", 2, 5, 1f,
+            null);
     sq.getDatasetSequence().addSequenceFeature(sf2);
     sfs = sq.getSequenceFeatures();
     assertEquals(1, sfs.length);
-- 
1.7.10.2