Merge pull request #998 from richarda23/#944-testcoverage

#944 testcoverage

biojava-core/src/main/java/org/biojava/nbio/core/sequence/AccessionID.java

@@ -26,7 +26,7 @@
 import org.biojava.nbio.core.util.Hashcoder;
 
 /**
- * Used in Sequences as the unique indentifier. If possible, set the {@link DataSource} to know the
+ * Used in Sequences as the unique identifier. If possible, set the {@link DataSource} to know the
  * source of the id. This allows a SequenceProxy to gather features or related sequences
  * Protein->Gene as an example. When parsing a Blast file it is also possible
  * to identify the type of ID
@@ -42,26 +42,21 @@ public class AccessionID {
 	private String identifier = null;
 
 	/**
-	 *
+	 * Default constructor sets id t empty string
 	 */
-
 	public AccessionID(){
 		id = "";
-
 	}
 
 	/**
-	 *
-	 * @param id
+	 * Creates an id with default DataSource.LOCAL source
+	 * @param id non-null
 	 */
 	public AccessionID(String id) {
-		this.id = id.trim();
-		this.source = DataSource.LOCAL;
+		this(id, DataSource.LOCAL);
 	}
 
-
 	/**
-	 *
 	 * @param id
 	 * @param source
 	 */
@@ -114,11 +109,6 @@ public int hashCode() {
 		return r;
 	}
 
-//   public void setDataSource(DataSource dataSource){
-//       source = dataSource;
-//   }
-
-
 	/**
 	 * In case if the {@link #getID() } is not unique keeps the id version.
 	 * @return the version
@@ -132,10 +122,8 @@ public void setVersion(Integer version) {
 	}
 
 	/**
-	 * In case if {@link #getID() } in not unique keeps the alternative id, eg. NCBI GI number.
-	 *
-	 * This may null.
-	 *
+	 * In case if {@link #getID() } is not unique, keeps the alternative id, e.g. NCBI GI number.
+	 * This may be null.
 	 * @return
 	 */
 	public String getIdentifier() {

biojava-core/src/main/java/org/biojava/nbio/core/sequence/CDSSequence.java

@@ -23,6 +23,7 @@
 package org.biojava.nbio.core.sequence;
 
 
+import org.biojava.nbio.core.exceptions.CompoundNotFoundException;
 import org.biojava.nbio.core.sequence.compound.DNACompoundSet;
 import org.biojava.nbio.core.sequence.compound.NucleotideCompound;
 import org.biojava.nbio.core.sequence.template.CompoundSet;
@@ -46,8 +47,15 @@ public class CDSSequence extends DNASequence {
 	 * @param bioBegin
 	 * @param bioEnd
 	 * @param phase
+	 * @throws  IllegalArgumentException if  parentSequence is incompatible with DNACompoundSet
 	 */
 	public CDSSequence(TranscriptSequence parentSequence, int bioBegin, int bioEnd, int phase) {
+		setCompoundSet(DNACompoundSet.getDNACompoundSet());
+		try {
+			initSequenceStorage(parentSequence.getSequenceAsString());
+		} catch (CompoundNotFoundException e) {
+			throw new IllegalArgumentException(e);
+		}
 		parentTranscriptSequence = parentSequence;
 		this.setParentSequence(parentTranscriptSequence);
 		setBioBegin(bioBegin);

biojava-core/src/main/java/org/biojava/nbio/core/sequence/ChromosomeSequence.java

@@ -125,13 +125,11 @@ public GeneSequence removeGeneSequence(String accession) {
 	 * which actually contains the sequence data. Strand is important for positive and negative
 	 * direction where negative strand means we need reverse complement. If negative strand then
 	 * bioBegin will be greater than bioEnd
-	 *
-	 *
 	 * @param accession
-	 * @param begin
-	 * @param end
+	 * @param bioBegin
+	 * @param bioEnd
 	 * @param strand
-	 * @return
+	 * @return A GeneSequence
 	 */
 	public GeneSequence addGene(AccessionID accession, int bioBegin, int bioEnd, Strand strand) {
 		GeneSequence geneSequence = new GeneSequence(this, bioBegin, bioEnd, strand);

biojava-core/src/main/java/org/biojava/nbio/core/sequence/ExonComparator.java

@@ -28,7 +28,7 @@
 
 
 /**
- * Sort Exon where it is a little confusing if exons shoud always be ordered left to right
+ * Sort Exon where it is a little confusing if exons should always be ordered left to right
  * where a negative stranded gene should go the other direction. Need to think about this?
  * @author Scooter Willis <willishf at gmail dot com>
  */
@@ -37,7 +37,6 @@ public class ExonComparator implements Comparator<ExonSequence>, Serializable{
 
 	@Override
 	public int compare(ExonSequence o1, ExonSequence o2) {
-
 		return o1.getBioBegin() - o2.getBioBegin();
 	}
 

biojava-core/src/main/java/org/biojava/nbio/core/sequence/ExonSequence.java

@@ -22,15 +22,15 @@
  */
 package org.biojava.nbio.core.sequence;
 
-
+import org.biojava.nbio.core.exceptions.CompoundNotFoundException;
+import org.biojava.nbio.core.sequence.compound.DNACompoundSet;
 
 /**
  * A gene contains a collection of Exon sequences
  * @author Scooter Willis
  */
 public class ExonSequence extends DNASequence {
 
-	//private static final Logger log = Logger.getLogger(ExonSequence.class.getName());
 
 	/**
 	 * Need a parent gene sequence and the bioBegin and bioEnd. An Exon sequence doesn't actually imply what the
@@ -44,6 +44,12 @@ public class ExonSequence extends DNASequence {
 	 * @param bioEnd
 	 */
 	public ExonSequence(GeneSequence parentGeneSequence, int bioBegin, int bioEnd) {
+		setCompoundSet(DNACompoundSet.getDNACompoundSet());
+		try {
+			initSequenceStorage(parentGeneSequence.getSequenceAsString());
+		} catch (CompoundNotFoundException e) {
+			throw new IllegalArgumentException(e);
+		}
 		this.setParentSequence(parentGeneSequence);
 		setBioBegin(bioBegin);
 		setBioEnd(bioEnd);

biojava-core/src/main/java/org/biojava/nbio/core/sequence/GeneSequence.java

@@ -50,6 +50,29 @@ public class GeneSequence extends DNASequence {
 	private Strand strand = Strand.UNDEFINED;
 	private ChromosomeSequence chromosomeSequence;
 
+	/**
+	 * Use GeneSequence(ChromosomeSequence parentSequence, AccessionID accessionId,  int begin, int end, Strand strand)
+	 * which mandates an accessionID.
+	 * @param parentSequence
+	 * @param begin
+	 * @param end inclusive of end
+	 * @param strand force a gene to have strand and transcription sequence will inherit
+	 * @deprecated
+	 */
+	public GeneSequence(ChromosomeSequence parentSequence,  int begin, int end, Strand strand) {
+		setCompoundSet(DNACompoundSet.getDNACompoundSet());
+		try {
+			initSequenceStorage(parentSequence.getSequenceAsString());
+		} catch (CompoundNotFoundException e) {
+			throw new IllegalArgumentException(e);
+		}
+		chromosomeSequence = parentSequence;
+		setParentSequence(parentSequence);
+		setBioBegin(begin);
+		setBioEnd(end);
+		setStrand(strand);
+	}
+
 	/**
 	 * A class that keeps track of the details of a GeneSequence which is difficult to properly model. Two important concepts that is difficult
 	 * to make everything flexible but still work. You can have GFF features that only describe Exons or Exons/Introns or CDS regions and one
@@ -60,18 +83,15 @@ public class GeneSequence extends DNASequence {
 	 *
 	 * This is also a key class in the biojava-3-genome module for reading and writing GFF3 files
 	 *
-	 * @param parentDNASequence
+	 * @param parentSequence
+	 * @param accessionId An identifier for the gene.
 	 * @param begin
-	 * @param end inclusive of end
+	 * @param end
 	 * @param strand force a gene to have strand and transcription sequence will inherit
 	 */
-	public GeneSequence(ChromosomeSequence parentSequence, int begin, int end, Strand strand) {
-		chromosomeSequence = parentSequence;
-		setParentSequence(parentSequence);
-		setBioBegin(begin);
-		setBioEnd(end);
-		setStrand(strand);
-		this.setCompoundSet(DNACompoundSet.getDNACompoundSet());
+	public GeneSequence(ChromosomeSequence parentSequence, AccessionID accessionId,  int begin, int end, Strand strand) {
+		this(parentSequence,begin,end,strand);
+		setAccession(accessionId);
 	}
 
 	/**
@@ -116,7 +136,8 @@ public void addIntronsUsingExons() throws Exception {
 		for (int i = 0; i < exonSequenceList.size() - 1; i++) {
 			ExonSequence exon1 = exonSequenceList.get(i);
 			ExonSequence exon2 = exonSequenceList.get(i + 1);
-			this.addIntron(new AccessionID(this.getAccession().getID() + "-" + "intron" + intronIndex), exon1.getBioEnd() - shift, exon2.getBioBegin() + shift);
+			AccessionID intronId= new AccessionID(this.getAccession().getID() + "-" + "intron" + intronIndex);
+			this.addIntron(intronId, exon1.getBioEnd() - shift, exon2.getBioBegin() + shift);
 			intronIndex++;
 		}
 
@@ -168,8 +189,6 @@ public LinkedHashMap<String, TranscriptSequence> getTranscripts() {
 	 * @return transcriptsequence
 	 */
 	public TranscriptSequence removeTranscript(String accession) {
-
-
 		return transcriptSequenceHashMap.remove(accession);
 	}
 
@@ -194,7 +213,7 @@ public TranscriptSequence addTranscript(AccessionID accession, int begin, int en
 	/**
 	 * Remove the intron by accession
 	 * @param accession
-	 * @return intron sequence
+	 * @return the removed intron sequence, or null if no intron with that accession exists.
 	 */
 	public IntronSequence removeIntron(String accession) {
 		for (IntronSequence intronSequence : intronSequenceList) {
@@ -272,25 +291,25 @@ public ExonSequence addExon(AccessionID accession, int begin, int end) {
 	}
 
 	/**
-	 * 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);
 	}
 
 	/**
 	 * Try to give method clarity where you want a DNASequence coding in the 5' to 3' direction
 	 * Returns the DNASequence representative of the 5' and 3' reading based on strand
-	 * @return dna sequence
+	 * @return dna sequence or null if sequence could not be generated.
 	 */
 	public DNASequence getSequence5PrimeTo3Prime() {
 		String sequence = getSequenceAsString(this.getBioBegin(), this.getBioEnd(), this.getStrand());
@@ -308,11 +327,11 @@ public DNASequence getSequence5PrimeTo3Prime() {
 		DNASequence dnaSequence = null;
 		try {
 			dnaSequence = new DNASequence(sequence.toUpperCase());
+		    dnaSequence.setAccession(new AccessionID(this.getAccession().getID()));
 		} catch (CompoundNotFoundException e) {
 			// this should not happen, the sequence is DNA originally, if it does, there's a bug somewhere
 			logger.error("Could not create new DNA sequence in getSequence5PrimeTo3Prime(). Error: {}",e.getMessage());
 		}
-		dnaSequence.setAccession(new AccessionID(this.getAccession().getID()));
 		return dnaSequence;
 	}
 }

biojava-core/src/main/java/org/biojava/nbio/core/sequence/MultipleSequenceAlignment.java

@@ -180,7 +180,7 @@ public String toString() {
 	// helper methods
 
 	/**
-	 * Helper method that does all the formating work
+	 * Helper method that does all the formatting work
 	 * @param width
 	 * @param header
 	 * @param idFormat

biojava-core/src/main/java/org/biojava/nbio/core/sequence/RNASequence.java

@@ -72,7 +72,7 @@ public RNASequence(String seqString, CompoundSet<NucleotideCompound> compoundSet
 	}
 
 	/**
-	 * Create a RNA sequence from a proxy reader and user defined RNA compound set
+	 * Create a RNA sequence from a proxy reader and user-defined RNA compound set
 	 * @param proxyLoader
 	 * @param compoundSet
 	 */
@@ -86,7 +86,7 @@ public RNASequence(ProxySequenceReader<NucleotideCompound> proxyLoader,
 	 * @return
 	 */
 	public SequenceView<NucleotideCompound> getReverseComplement() {
-		return new ComplementSequenceView<NucleotideCompound>(getInverse());
+		return new ComplementSequenceView<>(getInverse());
 	}
 
 	/**
@@ -97,15 +97,15 @@ public SequenceView<NucleotideCompound> getReverseComplement() {
 	 */
 	@Override
 public SequenceView<NucleotideCompound> getInverse() {
-		return new ReversedSequenceView<NucleotideCompound>(this);
+		return new ReversedSequenceView<>(this);
 	}
 
 	/**
 	 * Get the complement view of the RNA sequence
 	 * @return
 	 */
 	public SequenceView<NucleotideCompound> getComplement() {
-		return new ComplementSequenceView<NucleotideCompound>(this);
+		return new ComplementSequenceView<>(this);
 	}
 
 	/**
@@ -117,7 +117,7 @@ public ProteinSequence getProteinSequence() {
 	}
 
 	/**
-	 * Get the ProteinSequene from the RNA sequence with user defined
+	 * Get the ProteinSequence from the RNA sequence with user-defined
 	 * transcription engine
 	 *
 	 * @param engine

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{

biojava-core/src/main/java/org/biojava/nbio/core/sequence/TaxonomyID.java

@@ -26,14 +26,16 @@
 /**
  * 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;
 	DataSource dataSource = DataSource.UNKNOWN;
 
 	public TaxonomyID(String id, DataSource dataSource) {
+		// TODO should throw IAE if null args?
 		this.id = id;
 		this.dataSource = dataSource;
 	}