From 1145ab823ba8ffd317754588efeb259d70c1f4a1 Mon Sep 17 00:00:00 2001 From: gmungoc Date: Fri, 4 Nov 2016 10:02:19 +0000 Subject: [PATCH 1/1] JAL-966 JAL-1854 removed redundant methods / modifiers from search result interface, corrected faulty toString --- src/jalview/appletgui/AlignmentPanel.java | 2 +- src/jalview/appletgui/Finder.java | 11 +-- src/jalview/datamodel/SearchResultMatchI.java | 27 +++++-- src/jalview/datamodel/SearchResults.java | 95 +++++-------------------- src/jalview/datamodel/SearchResultsI.java | 63 +++++++--------- src/jalview/gui/AlignmentPanel.java | 2 +- src/jalview/gui/Finder.java | 11 +-- test/jalview/analysis/FinderTest.java | 36 +++++----- test/jalview/datamodel/MatchTest.java | 14 +--- test/jalview/datamodel/SearchResultsTest.java | 27 ++----- 10 files changed, 107 insertions(+), 181 deletions(-) diff --git a/src/jalview/appletgui/AlignmentPanel.java b/src/jalview/appletgui/AlignmentPanel.java index f77d6a1..e97c347 100644 --- a/src/jalview/appletgui/AlignmentPanel.java +++ b/src/jalview/appletgui/AlignmentPanel.java @@ -920,7 +920,7 @@ public class AlignmentPanel extends Panel implements AdjustmentListener, * mapped), we can make the scroll-to location a sequence above the one * actually mapped. */ - SequenceI mappedTo = sr.getResultSequence(0); + SequenceI mappedTo = sr.getResults().get(0).getSequence(); List seqs = av.getAlignment().getSequences(); /* diff --git a/src/jalview/appletgui/Finder.java b/src/jalview/appletgui/Finder.java index d18295c..d2fe69c 100644 --- a/src/jalview/appletgui/Finder.java +++ b/src/jalview/appletgui/Finder.java @@ -20,6 +20,7 @@ */ package jalview.appletgui; +import jalview.datamodel.SearchResultMatchI; import jalview.datamodel.SearchResultsI; import jalview.datamodel.SequenceFeature; import jalview.datamodel.SequenceI; @@ -116,13 +117,15 @@ public class Finder extends Panel implements ActionListener SequenceFeature[] features = new SequenceFeature[searchResults .getSize()]; - for (int i = 0; i < searchResults.getSize(); i++) + int i = 0; + for (SearchResultMatchI match : searchResults.getResults()) { - seqs[i] = searchResults.getResultSequence(i); + seqs[i] = match.getSequence().getDatasetSequence(); features[i] = new SequenceFeature(textfield.getText().trim(), - "Search Results", null, searchResults.getResultStart(i), - searchResults.getResultEnd(i), "Search Results"); + "Search Results", null, match.getStart(), match.getEnd(), + "Search Results"); + i++; } if (ap.seqPanel.seqCanvas.getFeatureRenderer().amendFeatures(seqs, diff --git a/src/jalview/datamodel/SearchResultMatchI.java b/src/jalview/datamodel/SearchResultMatchI.java index e886bed..732f1dc 100644 --- a/src/jalview/datamodel/SearchResultMatchI.java +++ b/src/jalview/datamodel/SearchResultMatchI.java @@ -1,17 +1,30 @@ package jalview.datamodel; +/** + * An interface that describes one matched region of an alignment, as one + * contiguous portion of a single dataset sequence + */ public interface SearchResultMatchI { + /** + * Returns the matched sequence + * + * @return + */ + SequenceI getSequence(); - public abstract SequenceI getSequence(); - - public abstract int getStart(); - - public abstract int getEnd(); + /** + * Returns the start position of the match in the sequence (base 1) + * + * @return + */ + int getStart(); /** - * Returns the string of characters in the matched region. + * Returns the end position of the match in the sequence (base 1) + * + * @return */ - public abstract String getCharacters(); + int getEnd(); } \ No newline at end of file diff --git a/src/jalview/datamodel/SearchResults.java b/src/jalview/datamodel/SearchResults.java index 2ba1612..1bf5475 100755 --- a/src/jalview/datamodel/SearchResults.java +++ b/src/jalview/datamodel/SearchResults.java @@ -21,7 +21,6 @@ package jalview.datamodel; import java.util.ArrayList; -import java.util.Arrays; import java.util.BitSet; import java.util.List; @@ -120,28 +119,18 @@ public class SearchResults implements SearchResultsI } /** - * Returns the string of characters in the matched region, prefixed by the - * start position, e.g. "12CGT" or "208K" + * Returns a representation as "seqid/start-end" */ @Override public String toString() { - final int from = Math.max(start - 1, 0); - String startPosition = String.valueOf(from); - return startPosition + getCharacters(); - } - - /* (non-Javadoc) - * @see jalview.datamodel.SearchResultMatchI#getCharacters() - */ - @Override - public String getCharacters() - { - char[] chars = sequence.getSequence(); - // convert start/end to base 0 (with bounds check) - final int from = Math.max(start - 1, 0); - final int to = Math.min(end, chars.length + 1); - return String.valueOf(Arrays.copyOfRange(chars, from, to)); + StringBuilder sb = new StringBuilder(); + if (sequence != null) + { + sb.append(sequence.getName()).append("/"); + } + sb.append(start).append("-").append(end); + return sb.toString(); } public void setSequence(SequenceI seq) @@ -198,12 +187,10 @@ public class SearchResults implements SearchResultsI public boolean involvesSequence(SequenceI sequence) { SequenceI ds = sequence.getDatasetSequence(); - Match m; for (SearchResultMatchI _m : matches) { - m = (Match) _m; - if (m.sequence != null - && (m.sequence == sequence || m.sequence == ds)) + SequenceI matched = _m.getSequence(); + if (matched != null && (matched == sequence || matched == ds)) { return true; } @@ -320,33 +307,6 @@ public class SearchResults implements SearchResultsI } /* (non-Javadoc) - * @see jalview.datamodel.SearchResultsI#getResultSequence(int) - */ - @Override - public SequenceI getResultSequence(int index) - { - return matches.get(index).getSequence(); - } - - /* (non-Javadoc) - * @see jalview.datamodel.SearchResultsI#getResultStart(int) - */ - @Override - public int getResultStart(int i) - { - return matches.get(i).getStart(); - } - - /* (non-Javadoc) - * @see jalview.datamodel.SearchResultsI#getResultEnd(int) - */ - @Override - public int getResultEnd(int i) - { - return matches.get(i).getEnd(); - } - - /* (non-Javadoc) * @see jalview.datamodel.SearchResultsI#isEmpty() */ @Override @@ -365,44 +325,23 @@ public class SearchResults implements SearchResultsI } /** - * Return the results as a string of characters (bases) prefixed by start - * position(s). Meant for use when the context ensures that all matches are to - * regions of the same sequence (otherwise the result is meaningless). + * Return the results as a list of matches [seq1/from-to, seq2/from-to, ...] * * @return */ @Override public String toString() { - StringBuilder result = new StringBuilder(256); - for (SearchResultMatchI m : matches) - { - result.append(m.toString()); - } - return result.toString(); - } - - /** - * Return the results as a string of characters (bases). Meant for use when - * the context ensures that all matches are to regions of the same sequence - * (otherwise the result is meaningless). - * - * @return - */ - public String getCharacters() - { - StringBuilder result = new StringBuilder(256); - for (SearchResultMatchI m : matches) - { - result.append(m.getCharacters()); - } - return result.toString(); + return matches == null ? "" : matches.toString(); } /** - * Hashcode is has derived from the list of matches. This ensures that when - * two SearchResults objects satisfy the test for equals(), then they have the + * Hashcode is derived from the list of matches. This ensures that when two + * SearchResults objects satisfy the test for equals(), then they have the * same hashcode. + * + * @see Match#hashCode() + * @see java.util.AbstractList#hashCode() */ @Override public int hashCode() diff --git a/src/jalview/datamodel/SearchResultsI.java b/src/jalview/datamodel/SearchResultsI.java index f90832a..93183f2 100644 --- a/src/jalview/datamodel/SearchResultsI.java +++ b/src/jalview/datamodel/SearchResultsI.java @@ -1,16 +1,17 @@ package jalview.datamodel; - import java.util.BitSet; import java.util.List; +/** + * An interface describing the result of a search or other operation which + * highlights matched regions of an alignment + */ public interface SearchResultsI { /** - * This method replaces the old search results which merely held an alignment - * index of search matches. This broke when sequences were moved around the - * alignment + * Adds one region to the results * * @param seq * Sequence @@ -18,68 +19,56 @@ public interface SearchResultsI * int * @param end * int - * @return SearchResultMatchI instance created for this instance + * @return */ - public abstract SearchResultMatchI addResult(SequenceI seq, int start, - int end); + SearchResultMatchI addResult(SequenceI seq, int start, int end); /** - * Quickly check if the given sequence is referred to in the search results + * Answers true if the search results include the given sequence (or its + * dataset sequence), else false * * @param sequence - * (specific alignment sequence or a dataset sequence) - * @return true if the results involve sequence + * @return */ - public abstract boolean involvesSequence(SequenceI sequence); + boolean involvesSequence(SequenceI sequence); /** - * This Method returns the search matches which lie between the start and end - * points of the sequence in question . It is optimised for returning objects - * for drawing on SequenceCanvas + * Returns an array of [from, to, from, to..] matched columns (base 0) between + * the given start and end columns of the given sequence. Returns null if no + * matches overlap the specified region. + *

+ * Implementations should provide an optimised method to return locations to + * highlight on a visible portion of an alignment. * * @param sequence - * sequence to highlight columns according to matches * @param start - * - first column of visible region + * first column of range (base 0, inclusive) * @param end - * - last column of visible region - * @return int[] ranges within start/end index on sequence - */ - public abstract int[] getResults(SequenceI sequence, int start, int end); - - public abstract int getSize(); - - public abstract SequenceI getResultSequence(int index); - - /** - * Returns the start position of the i'th match in the search results. - * - * @param i - * @return + * last column of range base 0, inclusive) + * @return int[] */ - public abstract int getResultStart(int i); + int[] getResults(SequenceI sequence, int start, int end); /** - * Returns the end position of the i'th match in the search results. + * Returns the number of matches found * - * @param i * @return */ - public abstract int getResultEnd(int i); + int getSize(); /** * Returns true if no search result matches are held. * * @return */ - public abstract boolean isEmpty(); + boolean isEmpty(); /** * Returns the list of matches. * * @return */ - public abstract List getResults(); + List getResults(); /** * Set bits in a bitfield for all columns in the given sequence collection @@ -91,5 +80,5 @@ public interface SearchResultsI * bitset to set * @return number of bits set */ - public abstract int markColumns(SequenceCollectionI sqcol, BitSet bs); + int markColumns(SequenceCollectionI sqcol, BitSet bs); } \ No newline at end of file diff --git a/src/jalview/gui/AlignmentPanel.java b/src/jalview/gui/AlignmentPanel.java index 26e0baa..9add6ef 100644 --- a/src/jalview/gui/AlignmentPanel.java +++ b/src/jalview/gui/AlignmentPanel.java @@ -1795,7 +1795,7 @@ public class AlignmentPanel extends GAlignmentPanel implements * mapped), we can make the scroll-to location a sequence above the one * actually mapped. */ - SequenceI mappedTo = sr.getResultSequence(0); + SequenceI mappedTo = sr.getResults().get(0).getSequence(); List seqs = av.getAlignment().getSequences(); /* diff --git a/src/jalview/gui/Finder.java b/src/jalview/gui/Finder.java index 5d039a1..a8dbc38 100755 --- a/src/jalview/gui/Finder.java +++ b/src/jalview/gui/Finder.java @@ -20,6 +20,7 @@ */ package jalview.gui; +import jalview.datamodel.SearchResultMatchI; import jalview.datamodel.SearchResultsI; import jalview.datamodel.SequenceFeature; import jalview.datamodel.SequenceI; @@ -208,13 +209,15 @@ public class Finder extends GFinder SequenceFeature[] features = new SequenceFeature[searchResults .getSize()]; - for (int i = 0; i < searchResults.getSize(); i++) + int i = 0; + for (SearchResultMatchI match : searchResults.getResults()) { - seqs[i] = searchResults.getResultSequence(i).getDatasetSequence(); + seqs[i] = match.getSequence().getDatasetSequence(); features[i] = new SequenceFeature(textfield.getText().trim(), - "Search Results", null, searchResults.getResultStart(i), - searchResults.getResultEnd(i), "Search Results"); + "Search Results", null, match.getStart(), match.getEnd(), + "Search Results"); + i++; } if (ap.getSeqPanel().seqCanvas.getFeatureRenderer().amendFeatures(seqs, diff --git a/test/jalview/analysis/FinderTest.java b/test/jalview/analysis/FinderTest.java index ef7c3a6..971a8c6 100644 --- a/test/jalview/analysis/FinderTest.java +++ b/test/jalview/analysis/FinderTest.java @@ -13,6 +13,8 @@ import jalview.gui.AlignFrame; import jalview.io.FileLoader; import jalview.io.FormatAdapter; +import java.util.List; + import org.testng.annotations.BeforeClass; import org.testng.annotations.Test; @@ -45,12 +47,13 @@ public class FinderTest // should match seq2 efH and seq3 EFH SearchResultsI sr = f.getSearchResults(); assertEquals(sr.getSize(), 2); - assertSame(al.getSequenceAt(1), sr.getResultSequence(0)); - assertSame(al.getSequenceAt(2), sr.getResultSequence(1)); - assertEquals(sr.getResultStart(0), 5); - assertEquals(sr.getResultEnd(0), 7); - assertEquals(sr.getResultStart(1), 4); - assertEquals(sr.getResultEnd(1), 6); + List matches = sr.getResults(); + assertSame(al.getSequenceAt(1), matches.get(0).getSequence()); + assertSame(al.getSequenceAt(2), matches.get(1).getSequence()); + assertEquals(matches.get(0).getStart(), 5); + assertEquals(matches.get(0).getEnd(), 7); + assertEquals(matches.get(1).getStart(), 4); + assertEquals(matches.get(1).getEnd(), 6); } /** @@ -66,12 +69,13 @@ public class FinderTest // seq1 and seq4 have 9 residues; no match in other sequences SearchResultsI sr = f.getSearchResults(); assertEquals(sr.getSize(), 2); - assertSame(al.getSequenceAt(0), sr.getResultSequence(0)); - assertSame(al.getSequenceAt(3), sr.getResultSequence(1)); - assertEquals(sr.getResultStart(0), 9); - assertEquals(sr.getResultEnd(0), 9); - assertEquals(sr.getResultStart(1), 9); - assertEquals(sr.getResultEnd(1), 9); + List matches = sr.getResults(); + assertSame(al.getSequenceAt(0), matches.get(0).getSequence()); + assertSame(al.getSequenceAt(3), matches.get(1).getSequence()); + assertEquals(matches.get(0).getStart(), 9); + assertEquals(matches.get(0).getEnd(), 9); + assertEquals(matches.get(1).getStart(), 9); + assertEquals(matches.get(1).getEnd(), 9); } /** @@ -97,10 +101,10 @@ public class FinderTest f.find("e"); // matches in sequence assertTrue(f.getIdMatch().isEmpty()); assertEquals(f.getSearchResults().getSize(), 1); - assertEquals(f.getSearchResults().getResultStart(0), 5); - assertEquals(f.getSearchResults().getResultEnd(0), 5); - assertSame(f.getSearchResults().getResultSequence(0), - al.getSequenceAt(1)); + List matches = f.getSearchResults().getResults(); + assertEquals(matches.get(0).getStart(), 5); + assertEquals(matches.get(0).getEnd(), 5); + assertSame(matches.get(0).getSequence(), al.getSequenceAt(1)); // still in the second sequence assertEquals(f.getSeqIndex(), 1); // next residue position to search from is 5 diff --git a/test/jalview/datamodel/MatchTest.java b/test/jalview/datamodel/MatchTest.java index 65c894f..95764d0 100644 --- a/test/jalview/datamodel/MatchTest.java +++ b/test/jalview/datamodel/MatchTest.java @@ -24,8 +24,6 @@ import static org.testng.AssertJUnit.assertEquals; import static org.testng.AssertJUnit.assertFalse; import static org.testng.AssertJUnit.assertTrue; -import jalview.datamodel.SearchResults.Match; - import org.testng.annotations.Test; public class MatchTest @@ -34,17 +32,9 @@ public class MatchTest @Test(groups = { "Functional" }) public void testToString() { - SequenceI seq = new Sequence("", "abcdefghijklm"); - SearchResultMatchI m = new SearchResults().new Match(seq, 3, 5); - assertEquals("2cde", m.toString()); - } - - @Test(groups = { "Functional" }) - public void testGetCharacters() - { - SequenceI seq = new Sequence("", "abcdefghijklm"); + SequenceI seq = new Sequence("Seq1", "abcdefghijklm"); SearchResultMatchI m = new SearchResults().new Match(seq, 3, 5); - assertEquals("cde", m.getCharacters()); + assertEquals("Seq1/3-5", m.toString()); } @Test(groups = { "Functional" }) diff --git a/test/jalview/datamodel/SearchResultsTest.java b/test/jalview/datamodel/SearchResultsTest.java index 65553c9..19e89d2 100644 --- a/test/jalview/datamodel/SearchResultsTest.java +++ b/test/jalview/datamodel/SearchResultsTest.java @@ -36,31 +36,16 @@ public class SearchResultsTest @Test(groups = { "Functional" }) public void testToString() { - SequenceI seq = new Sequence("", "abcdefghijklm"); + SequenceI seq = new Sequence("Seq1", "abcdefghijklm"); SearchResultsI sr = new SearchResults(); sr.addResult(seq, 1, 1); - assertEquals("0a", sr.toString()); + assertEquals("[Seq1/1-1]", sr.toString()); sr.addResult(seq, 3, 5); - assertEquals("0a2cde", sr.toString()); + assertEquals("[Seq1/1-1, Seq1/3-5]", sr.toString()); - seq = new Sequence("", "pqrstuvwxy"); + seq = new Sequence("Seq2", "pqrstuvwxy"); sr.addResult(seq, 6, 7); - assertEquals("0a2cde5uv", sr.toString()); - } - - @Test(groups = { "Functional" }) - public void testGetCharacters() - { - SequenceI seq = new Sequence("", "abcdefghijklm"); - SearchResults sr = new SearchResults(); - sr.addResult(seq, 1, 1); - assertEquals("a", sr.getCharacters()); - sr.addResult(seq, 3, 5); - assertEquals("acde", sr.getCharacters()); - - seq = new Sequence("", "pqrstuvwxy"); - sr.addResult(seq, 6, 7); - assertEquals("acdeuv", sr.getCharacters()); + assertEquals("[Seq1/1-1, Seq1/3-5, Seq2/6-7]", sr.toString()); } @Test(groups = { "Functional" }) @@ -77,7 +62,7 @@ public class SearchResultsTest assertTrue(sr2.equals(sr1)); // reflexive /* - * only one result is not empty + * if only one result is not empty */ sr1.addResult(seq1, 1, 1); assertTrue(sr1.equals(sr1)); -- 1.7.10.2