From bb6c9d30f57964136b759376b121f121f61c11e6 Mon Sep 17 00:00:00 2001
From: gmungoc <g.m.carstairs@dundee.ac.uk>
Date: Sun, 24 Feb 2019 08:10:17 +0000
Subject: [PATCH] JAL-3198 SequenceGroup 'bulk constructor' without duplicates
 check

---
 src/jalview/datamodel/SequenceGroup.java      |   14 ++++++++-
 src/jalview/gui/AlignFrame.java               |    8 ++----
 src/jalview/gui/ScalePanel.java               |   11 +------
 test/jalview/datamodel/SequenceGroupTest.java |   38 +++++++++++++++++++++++--
 4 files changed, 52 insertions(+), 19 deletions(-)

diff --git a/src/jalview/datamodel/SequenceGroup.java b/src/jalview/datamodel/SequenceGroup.java
index fc8ac49..3bf7bc4 100755
--- a/src/jalview/datamodel/SequenceGroup.java
+++ b/src/jalview/datamodel/SequenceGroup.java
@@ -88,7 +88,7 @@ public class SequenceGroup implements AnnotatedCollectionI
   /**
    * group members
    */
-  private List<SequenceI> sequences = new ArrayList<>();
+  private List<SequenceI> sequences;
 
   /**
    * representative sequence for this group (if any)
@@ -162,6 +162,7 @@ public class SequenceGroup implements AnnotatedCollectionI
   {
     groupName = "JGroup:" + this.hashCode();
     cs = new ResidueShader();
+    sequences = new ArrayList<>();
   }
 
   /**
@@ -243,6 +244,17 @@ public class SequenceGroup implements AnnotatedCollectionI
     }
   }
 
+  /**
+   * Constructor that copies the given list of sequences
+   * 
+   * @param seqs
+   */
+  public SequenceGroup(List<SequenceI> seqs)
+  {
+    this();
+    this.sequences.addAll(seqs);
+  }
+
   public boolean isShowSequenceLogo()
   {
     return showSequenceLogo;
diff --git a/src/jalview/gui/AlignFrame.java b/src/jalview/gui/AlignFrame.java
index 86b4422..936e714 100644
--- a/src/jalview/gui/AlignFrame.java
+++ b/src/jalview/gui/AlignFrame.java
@@ -2398,12 +2398,8 @@ public class AlignFrame extends GAlignFrame implements DropTargetListener,
   @Override
   public void selectAllSequenceMenuItem_actionPerformed(ActionEvent e)
   {
-    SequenceGroup sg = new SequenceGroup();
-
-    for (int i = 0; i < viewport.getAlignment().getSequences().size(); i++)
-    {
-      sg.addSequence(viewport.getAlignment().getSequenceAt(i), false);
-    }
+    SequenceGroup sg = new SequenceGroup(
+            viewport.getAlignment().getSequences());
 
     sg.setEndRes(viewport.getAlignment().getWidth() - 1);
     viewport.setSelectionGroup(sg);
diff --git a/src/jalview/gui/ScalePanel.java b/src/jalview/gui/ScalePanel.java
index 6abee08..5821b61 100755
--- a/src/jalview/gui/ScalePanel.java
+++ b/src/jalview/gui/ScalePanel.java
@@ -23,7 +23,6 @@ package jalview.gui;
 import jalview.datamodel.ColumnSelection;
 import jalview.datamodel.HiddenColumns;
 import jalview.datamodel.SequenceGroup;
-import jalview.datamodel.SequenceI;
 import jalview.renderer.ScaleRenderer;
 import jalview.renderer.ScaleRenderer.ScaleMark;
 import jalview.util.MessageManager;
@@ -237,15 +236,7 @@ public class ScalePanel extends JPanel
     }
 
     av.getColumnSelection().addElement(res);
-    SequenceGroup sg = new SequenceGroup();
-    // try to be as quick as possible
-    SequenceI[] iVec = av.getAlignment().getSequencesArray();
-    for (int i = 0; i < iVec.length; i++)
-    {
-      sg.addSequence(iVec[i], false);
-      iVec[i] = null;
-    }
-    iVec = null;
+    SequenceGroup sg = new SequenceGroup(av.getAlignment().getSequences());
     sg.setStartRes(res);
     sg.setEndRes(res);
 
diff --git a/test/jalview/datamodel/SequenceGroupTest.java b/test/jalview/datamodel/SequenceGroupTest.java
index b0af5c8..8419d4c 100644
--- a/test/jalview/datamodel/SequenceGroupTest.java
+++ b/test/jalview/datamodel/SequenceGroupTest.java
@@ -14,12 +14,14 @@ import jalview.schemes.NucleotideColourScheme;
 import jalview.schemes.PIDColourScheme;
 
 import java.awt.Color;
+import java.util.ArrayList;
 import java.util.Collections;
-
-import junit.extensions.PA;
+import java.util.List;
 
 import org.testng.annotations.Test;
 
+import junit.extensions.PA;
+
 public class SequenceGroupTest
 {
   @Test(groups={"Functional"})
@@ -301,4 +303,36 @@ public class SequenceGroupTest
      */
     assertNull(sg2.getContext());
   }
+
+  @Test(groups = { "Functional" })
+  public void testConstructor_list()
+  {
+    SequenceI s1 = new Sequence("abcde", "fg");
+    SequenceI s2 = new Sequence("foo", "bar");
+    List<SequenceI> seqs = new ArrayList<SequenceI>();
+    seqs.add(s1);
+    seqs.add(s2);
+    SequenceGroup sg = new SequenceGroup(seqs);
+
+    /*
+     * verify sg has a copy of the original list
+     */
+    List<SequenceI> sgList = sg.getSequences();
+    assertNotSame(sgList, seqs);
+    assertEquals(sgList, seqs);
+
+    /*
+     * add to sgList, original is unchanged
+     */
+    sg.addSequence(new Sequence("bar", "foo"), false);
+    assertEquals(sgList.size(), 3);
+    assertEquals(seqs.size(), 2);
+
+    /*
+     * delete from original, sgList is unchanged
+     */
+    seqs.remove(s1);
+    assertEquals(sgList.size(), 3);
+    assertEquals(seqs.size(), 1);
+  }
 }
-- 
1.7.10.2