From 2459e383f02f6ef9b15a1d6312fff89f74947a6e Mon Sep 17 00:00:00 2001 From: gmungoc Date: Thu, 12 Apr 2018 16:28:41 +0100 Subject: [PATCH] JAL-2223 don't add duplicate mappings to StructureSelectionManager --- src/jalview/structure/StructureMapping.java | 116 ++++++++++++++++++-- .../structure/StructureSelectionManager.java | 10 +- src/jalview/util/MapList.java | 21 +++- test/jalview/structure/StructureMappingTest.java | 81 +++++++++++++- test/jalview/util/MapListTest.java | 10 +- 5 files changed, 217 insertions(+), 21 deletions(-) diff --git a/src/jalview/structure/StructureMapping.java b/src/jalview/structure/StructureMapping.java index fcf322d..4174f5b 100644 --- a/src/jalview/structure/StructureMapping.java +++ b/src/jalview/structure/StructureMapping.java @@ -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 mapping; jalview.datamodel.Mapping seqToPdbMapping = null; + /** * Constructor * @@ -145,7 +146,7 @@ public class StructureMapping */ public List getPDBResNumRanges(int fromSeqPos, int toSeqPos) { - List result = new ArrayList(); + List 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; + } } diff --git a/src/jalview/structure/StructureSelectionManager.java b/src/jalview/structure/StructureSelectionManager.java index cdb4e0a..d68f346 100644 --- a/src/jalview/structure/StructureSelectionManager.java +++ b/src/jalview/structure/StructureSelectionManager.java @@ -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); + } } /** diff --git a/src/jalview/util/MapList.java b/src/jalview/util/MapList.java index 4658724..ae530f9 100644 --- a/src/jalview/util/MapList.java +++ b/src/jalview/util/MapList.java @@ -77,8 +77,8 @@ public class MapList */ public MapList() { - fromShifts = new ArrayList(); - toShifts = new ArrayList(); + 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 merged = new ArrayList(); + List 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 ranges = new ArrayList(); + List ranges = new ArrayList<>(); if (fs <= fe) { intv = fs; diff --git a/test/jalview/structure/StructureMappingTest.java b/test/jalview/structure/StructureMappingTest.java index f26c5f1..036aeaa 100644 --- a/test/jalview/structure/StructureMappingTest.java +++ b/test/jalview/structure/StructureMappingTest.java @@ -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 map = new HashMap(); + HashMap 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 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)); + } } diff --git a/test/jalview/util/MapListTest.java b/test/jalview/util/MapListTest.java index a2f38e2..95d7efe 100644 --- a/test/jalview/util/MapListTest.java +++ b/test/jalview/util/MapListTest.java @@ -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 ranges = new ArrayList(); + List 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 ranges = new ArrayList(); + List 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 ranges = new ArrayList(); + List 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 ranges = new ArrayList(); + List ranges = new ArrayList<>(); ranges.add(new int[] { 1, 3 }); ranges.add(new int[] { 2, 5 }); -- 1.7.10.2