JAL-2223 don't add duplicate mappings to StructureSelectionManager
authorgmungoc <g.m.carstairs@dundee.ac.uk>
Thu, 12 Apr 2018 15:28:41 +0000 (16:28 +0100)
committergmungoc <g.m.carstairs@dundee.ac.uk>
Thu, 12 Apr 2018 15:28:41 +0000 (16:28 +0100)
src/jalview/structure/StructureMapping.java
src/jalview/structure/StructureSelectionManager.java
src/jalview/util/MapList.java
test/jalview/structure/StructureMappingTest.java
test/jalview/util/MapListTest.java

index fcf322d..4174f5b 100644 (file)
@@ -30,6 +30,12 @@ import java.util.List;
 
 public class StructureMapping
 {
+  public static final int UNASSIGNED_VALUE = Integer.MIN_VALUE;
+
+  private static final int PDB_RES_NUM_INDEX = 0;
+
+  private static final int PDB_ATOM_NUM_INDEX = 1;
+
   String mappingDetails;
 
   SequenceI sequence;
@@ -40,17 +46,12 @@ public class StructureMapping
 
   String pdbchain;
 
-  public static final int UNASSIGNED_VALUE = Integer.MIN_VALUE;
-
-  private static final int PDB_RES_NUM_INDEX = 0;
-
-  private static final int PDB_ATOM_NUM_INDEX = 1;
-
   // Mapping key is residue index while value is an array containing PDB resNum,
   // and atomNo
   HashMap<Integer, int[]> mapping;
 
   jalview.datamodel.Mapping seqToPdbMapping = null;
+
   /**
    * Constructor
    * 
@@ -145,7 +146,7 @@ public class StructureMapping
    */
   public List<int[]> getPDBResNumRanges(int fromSeqPos, int toSeqPos)
   {
-    List<int[]> result = new ArrayList<int[]>();
+    List<int[]> result = new ArrayList<>();
     int startRes = -1;
     int endRes = -1;
 
@@ -263,4 +264,105 @@ public class StructureMapping
   {
     return seqToPdbMapping;
   }
+
+  /**
+   * A hash function that satisfies the contract that if two mappings are
+   * equal(), they have the same hashCode
+   */
+  @Override
+  public int hashCode()
+  {
+    final int prime = 31;
+    int result = 1;
+    result = prime * result
+            + ((mappingDetails == null) ? 0 : mappingDetails.hashCode());
+    result = prime * result
+            + ((pdbchain == null) ? 0 : pdbchain.hashCode());
+    result = prime * result + ((pdbfile == null) ? 0 : pdbfile.hashCode());
+    result = prime * result + ((pdbid == null) ? 0 : pdbid.hashCode());
+    result = prime * result
+            + ((seqToPdbMapping == null) ? 0 : seqToPdbMapping.hashCode());
+    result = prime * result
+            + ((sequence == null) ? 0 : sequence.hashCode());
+    return result;
+  }
+
+  @Override
+  public boolean equals(Object obj)
+  {
+    if (this == obj)
+    {
+      return true;
+    }
+    if (obj == null)
+    {
+      return false;
+    }
+    if (getClass() != obj.getClass())
+    {
+      return false;
+    }
+    StructureMapping other = (StructureMapping) obj;
+    if (mappingDetails == null)
+    {
+      if (other.mappingDetails != null)
+      {
+        return false;
+      }
+    }
+    else if (!mappingDetails.equals(other.mappingDetails))
+    {
+      return false;
+    }
+    if (pdbchain == null)
+    {
+      if (other.pdbchain != null)
+      {
+        return false;
+      }
+    }
+    else if (!pdbchain.equals(other.pdbchain))
+    {
+      return false;
+    }
+    if (pdbfile == null)
+    {
+      if (other.pdbfile != null)
+      {
+        return false;
+      }
+    }
+    else if (!pdbfile.equals(other.pdbfile))
+    {
+      return false;
+    }
+    if (pdbid == null)
+    {
+      if (other.pdbid != null)
+      {
+        return false;
+      }
+    }
+    else if (!pdbid.equals(other.pdbid))
+    {
+      return false;
+    }
+    if (seqToPdbMapping == null)
+    {
+      if (other.seqToPdbMapping != null)
+      {
+        return false;
+      }
+    }
+    else if (!seqToPdbMapping.equals(other.seqToPdbMapping))
+    {
+      return false;
+    }
+    if (sequence != other.sequence)
+    {
+      return false;
+    }
+
+    return true;
+  }
 }
index cdb4e0a..d68f346 100644 (file)
@@ -606,7 +606,10 @@ public class StructureSelectionManager
       }
       if (forStructureView)
       {
-        mappings.addAll(seqToStrucMapping);
+        for (StructureMapping sm : seqToStrucMapping)
+        {
+          addStructureMapping(sm); // not addAll!
+        }
       }
       if (progress != null)
       {
@@ -658,7 +661,10 @@ public class StructureSelectionManager
 
   public void addStructureMapping(StructureMapping sm)
   {
-    mappings.add(sm);
+    if (!mappings.contains(sm))
+    {
+      mappings.add(sm);
+    }
   }
 
   /**
index 4658724..ae530f9 100644 (file)
@@ -77,8 +77,8 @@ public class MapList
    */
   public MapList()
   {
-    fromShifts = new ArrayList<int[]>();
-    toShifts = new ArrayList<int[]>();
+    fromShifts = new ArrayList<>();
+    toShifts = new ArrayList<>();
   }
 
   /**
@@ -116,8 +116,17 @@ public class MapList
   {
     int hashCode = 31 * fromRatio;
     hashCode = 31 * hashCode + toRatio;
-    hashCode = 31 * hashCode + fromShifts.toArray().hashCode();
-    hashCode = 31 * hashCode + toShifts.toArray().hashCode();
+    for (int[] shift : fromShifts)
+    {
+      hashCode = 31 * hashCode + shift[0];
+      hashCode = 31 * hashCode + shift[1];
+    }
+    for (int[] shift : toShifts)
+    {
+      hashCode = 31 * hashCode + shift[0];
+      hashCode = 31 * hashCode + shift[1];
+    }
+
     return hashCode;
   }
 
@@ -347,7 +356,7 @@ public class MapList
     }
 
     boolean changed = false;
-    List<int[]> merged = new ArrayList<int[]>();
+    List<int[]> merged = new ArrayList<>();
     int[] lastRange = ranges.get(0);
     int lastDirection = lastRange[1] >= lastRange[0] ? 1 : -1;
     lastRange = new int[] { lastRange[0], lastRange[1] };
@@ -803,7 +812,7 @@ public class MapList
     {
       return null;
     }
-    List<int[]> ranges = new ArrayList<int[]>();
+    List<int[]> ranges = new ArrayList<>();
     if (fs <= fe)
     {
       intv = fs;
index f26c5f1..036aeaa 100644 (file)
@@ -1,8 +1,14 @@
 package jalview.structure;
 
 import static org.testng.Assert.assertEquals;
+import static org.testng.Assert.assertFalse;
 import static org.testng.Assert.assertTrue;
 
+import jalview.datamodel.Mapping;
+import jalview.datamodel.Sequence;
+import jalview.datamodel.SequenceI;
+import jalview.util.MapList;
+
 import java.util.HashMap;
 import java.util.List;
 
@@ -11,9 +17,9 @@ import org.testng.annotations.Test;
 public class StructureMappingTest
 {
   @Test(groups = "Functional")
-  public void testgetPDBResNumRanges()
+  public void testGetPDBResNumRanges()
   {
-    HashMap<Integer, int[]> map = new HashMap<Integer, int[]>();
+    HashMap<Integer, int[]> map = new HashMap<>();
 
     StructureMapping mapping = new StructureMapping(null, null, null, null,
             map, null);
@@ -43,4 +49,75 @@ public class StructureMappingTest
     assertEquals(ranges.get(1)[0], 15);
     assertEquals(ranges.get(1)[1], 15);
   }
+
+  @Test(groups = "Functional")
+  public void testEquals()
+  {
+    SequenceI seq1 = new Sequence("seq1", "ABCDE");
+    SequenceI seq2 = new Sequence("seq1", "ABCDE");
+    String pdbFile = "a/b/file1.pdb";
+    String pdbId = "1a70";
+    String chain = "A";
+    String mappingDetails = "these are the mapping details, honest";
+    HashMap<Integer, int[]> map = new HashMap<>();
+
+    Mapping seqToPdbMapping = new Mapping(seq1,
+            new MapList(new int[]
+            { 1, 5 }, new int[] { 2, 6 }, 1, 1));
+    StructureMapping sm1 = new StructureMapping(seq1, pdbFile, pdbId, chain,
+            map, mappingDetails, seqToPdbMapping);
+    assertFalse(sm1.equals(null));
+    assertFalse(sm1.equals("x"));
+
+    StructureMapping sm2 = new StructureMapping(seq1, pdbFile, pdbId, chain,
+            map, mappingDetails, seqToPdbMapping);
+    assertTrue(sm1.equals(sm2));
+    assertTrue(sm2.equals(sm1));
+    assertEquals(sm1.hashCode(), sm2.hashCode());
+
+    // with different sequence
+    sm2 = new StructureMapping(seq2, pdbFile, pdbId, chain, map,
+            mappingDetails, seqToPdbMapping);
+    assertFalse(sm1.equals(sm2));
+    assertFalse(sm2.equals(sm1));
+
+    // with different file
+    sm2 = new StructureMapping(seq1, "a/b/file2.pdb", pdbId, chain, map,
+            mappingDetails, seqToPdbMapping);
+    assertFalse(sm1.equals(sm2));
+    assertFalse(sm2.equals(sm1));
+
+    // with different pdbid (case sensitive)
+    sm2 = new StructureMapping(seq1, pdbFile, "1A70", chain, map,
+            mappingDetails, seqToPdbMapping);
+    assertFalse(sm1.equals(sm2));
+    assertFalse(sm2.equals(sm1));
+
+    // with different chain
+    sm2 = new StructureMapping(seq1, pdbFile, pdbId, "B", map,
+            mappingDetails, seqToPdbMapping);
+    assertFalse(sm1.equals(sm2));
+    assertFalse(sm2.equals(sm1));
+
+    // map is ignore for this test
+    sm2 = new StructureMapping(seq1, pdbFile, pdbId, chain, null,
+            mappingDetails, seqToPdbMapping);
+    assertTrue(sm1.equals(sm2));
+    assertTrue(sm2.equals(sm1));
+
+    // with different mapping details
+    sm2 = new StructureMapping(seq1, pdbFile, pdbId, chain, map,
+            "different details!", seqToPdbMapping);
+    assertFalse(sm1.equals(sm2));
+    assertFalse(sm2.equals(sm1));
+
+    // with different seq to pdb mapping
+    Mapping map2 = new Mapping(seq1,
+            new MapList(new int[]
+            { 1, 5 }, new int[] { 3, 7 }, 1, 1));
+    sm2 = new StructureMapping(seq1, pdbFile, pdbId, chain, map,
+            mappingDetails, map2);
+    assertFalse(sm1.equals(sm2));
+    assertFalse(sm2.equals(sm1));
+  }
 }
index a2f38e2..95d7efe 100644 (file)
@@ -391,7 +391,9 @@ public class MapListTest
     MapList ml7 = new MapList(codons, protein, 3, 1); // toShifts differ
 
     assertTrue(ml.equals(ml));
+    assertEquals(ml.hashCode(), ml.hashCode());
     assertTrue(ml.equals(ml1));
+    assertEquals(ml.hashCode(), ml1.hashCode());
     assertTrue(ml1.equals(ml));
 
     assertFalse(ml.equals(null));
@@ -426,7 +428,7 @@ public class MapListTest
   @Test(groups = { "Functional" })
   public void testGetRanges()
   {
-    List<int[]> ranges = new ArrayList<int[]>();
+    List<int[]> ranges = new ArrayList<>();
     ranges.add(new int[] { 2, 3 });
     ranges.add(new int[] { 5, 6 });
     assertEquals("[2, 3, 5, 6]", Arrays.toString(MapList.getRanges(ranges)));
@@ -603,7 +605,7 @@ public class MapListTest
   public void testAddRange()
   {
     int[] range = { 1, 5 };
-    List<int[]> ranges = new ArrayList<int[]>();
+    List<int[]> ranges = new ArrayList<>();
 
     // add to empty list:
     MapList.addRange(range, ranges);
@@ -702,7 +704,7 @@ public class MapListTest
   public void testCoalesceRanges()
   {
     assertNull(MapList.coalesceRanges(null));
-    List<int[]> ranges = new ArrayList<int[]>();
+    List<int[]> ranges = new ArrayList<>();
     assertSame(ranges, MapList.coalesceRanges(ranges));
     ranges.add(new int[] { 1, 3 });
     assertSame(ranges, MapList.coalesceRanges(ranges));
@@ -763,7 +765,7 @@ public class MapListTest
   @Test(groups = { "Functional" })
   public void testCoalesceRanges_withOverlap()
   {
-    List<int[]> ranges = new ArrayList<int[]>();
+    List<int[]> ranges = new ArrayList<>();
     ranges.add(new int[] { 1, 3 });
     ranges.add(new int[] { 2, 5 });