fix mutable collections in GeneSequence

otter606 · otter606 · commit 508845609636 · 2021-11-26T17:43:05.000Z
diff --git a/biojava-core/src/main/java/org/biojava/nbio/core/sequence/AccessionID.java b/biojava-core/src/main/java/org/biojava/nbio/core/sequence/AccessionID.java
@@ -53,8 +53,7 @@ public AccessionID(){
 	 * @param id non-null
 	 */
 	public AccessionID(String id) {
-		this.id = id.trim();
-		this.source = DataSource.LOCAL;
+		this(id, DataSource.LOCAL);
 	}
 
 	/**
diff --git a/biojava-core/src/main/java/org/biojava/nbio/core/sequence/GeneSequence.java b/biojava-core/src/main/java/org/biojava/nbio/core/sequence/GeneSequence.java
@@ -290,19 +290,19 @@ public ExonSequence addExon(AccessionID accession, int begin, int end) throws Ex
 	}
 
 	/**
-	 * Get the exons as an ArrayList
+	 * Get the exons as an ArrayList. Modifying this list will not modify the underlying collection
 	 * @return exons
 	 */
 	public ArrayList<ExonSequence> getExonSequences() {
-		return exonSequenceList;
+		return new ArrayList<>(exonSequenceList);
 	}
 
 	/**
-	 * Get the introns as an ArrayList
+	 * Get the introns as an ArrayList. Modifying this list will not modify the underlying collection
 	 * @return introns
 	 */
 	public ArrayList<IntronSequence> getIntronSequences() {
-		return intronSequenceList;
+		return  new ArrayList<>(intronSequenceList);
 	}
 
 	/**
diff --git a/biojava-core/src/main/java/org/biojava/nbio/core/sequence/SequenceComparator.java b/biojava-core/src/main/java/org/biojava/nbio/core/sequence/SequenceComparator.java
@@ -28,7 +28,7 @@
 import java.util.Comparator;
 
 /**
- * Used to sort sequences
+ * Used to sort sequences in ascending order of  bioBegin property.
  * @author Scooter Willis <willishf at gmail dot com>
  */
 public class SequenceComparator implements Comparator<AbstractSequence<?>>, Serializable{
diff --git a/biojava-core/src/main/java/org/biojava/nbio/core/sequence/TaxonomyID.java b/biojava-core/src/main/java/org/biojava/nbio/core/sequence/TaxonomyID.java
@@ -26,8 +26,10 @@
 /**
  * A sequence can be associated with a species or Taxonomy ID
  * @author Scooter Willis
+ *
  */
 public class TaxonomyID {
+   //TODO this should implement equals and hashcode if is value object?
 
 
 	private String id = null;
diff --git a/biojava-core/src/test/java/org/biojava/nbio/core/sequence/GeneSequenceTest.java b/biojava-core/src/test/java/org/biojava/nbio/core/sequence/GeneSequenceTest.java
@@ -6,6 +6,7 @@
 import org.junit.jupiter.api.Nested;
 import org.junit.jupiter.api.Test;
 
+import java.util.ArrayList;
 import java.util.List;
 
 import static org.junit.jupiter.api.Assertions.*;
@@ -114,12 +115,23 @@ void addRemoveExon() throws Exception {
     }
 
    @Test
-    void returnedCollectionsAreMutable() throws Exception {
+    void returnedExonCollectionsAreNotMutable() throws Exception {
        geneSequence = new GeneSequence(chromosomeSequence, new AccessionID("geneId"), 5,150, Strand.POSITIVE);
        geneSequence.addExon(new AccessionID("a"), 20, 50);
        List<ExonSequence> exons  =  geneSequence.getExonSequences();
        // this breaks encapsulation of the collections
        exons.remove(0);
-       assertEquals(0, geneSequence.getExonSequences().size());
+       assertEquals(1, geneSequence.getExonSequences().size());
    }
+    @Test
+    void returnedIntronCollectionsAreNotMutable() throws Exception {
+        geneSequence = SequenceTestUtils.anyGeneSequence();
+        geneSequence.addExon(new AccessionID("a"), 20, 50);
+        geneSequence.addExon(new AccessionID("b"), 80, 100);
+        geneSequence.addIntronsUsingExons();
+        ArrayList<IntronSequence> introns  =  geneSequence.getIntronSequences();
+        assertEquals(1, introns.size());
+        introns.remove(0);
+        assertEquals(1, geneSequence.getIntronSequences().size());
+    }
 }
diff --git a/biojava-core/src/test/java/org/biojava/nbio/core/sequence/TranscriptSequenceTest.java b/biojava-core/src/test/java/org/biojava/nbio/core/sequence/TranscriptSequenceTest.java
@@ -42,7 +42,6 @@ void CDSListIsEmpty() {
         }
 
         @Test
-        // whether it's -ve or +ve doesn't affect equals?
         void equals() {
             assertTrue(transcriptSeq.equals(transcriptSeq));
         }

Original file line number	Diff line number	Diff line change
`@@ -53,8 +53,7 @@ public AccessionID(){`
`53`	`53`	`* @param id non-null`
`54`	`54`	`*/`
`55`	`55`	`public AccessionID(String id) {`
`56`		`- this.id = id.trim();`
`57`		`- this.source = DataSource.LOCAL;`
	`56`	`+ this(id, DataSource.LOCAL);`
`58`	`57`	`}`
`59`	`58`
`60`	`59`	`/**`
Original file line number	Diff line number	Diff line change
`@@ -42,7 +42,6 @@ void CDSListIsEmpty() {`
`42`	`42`	`}`
`43`	`43`
`44`	`44`	`@Test`
`45`		`- // whether it's -ve or +ve doesn't affect equals?`
`46`	`45`	`void equals() {`
`47`	`46`	`assertTrue(transcriptSeq.equals(transcriptSeq));`
`48`	`47`	`}`