#3 EMBL support #8
Conversation
| private EncodingScheme encodingSheme; | ||
|
|
||
| /** | ||
| * Initialize a new FastaSequenceFactory with default encoding scheme |
| } | ||
|
|
||
| /** | ||
| * Initialize a new FastaSequenceFactory with a custom encoding scheme |
| /** | ||
| * Initialize a new FastaSequenceFactory with default encoding scheme | ||
| */ | ||
| public EmblSequenceFactory() { |
There was a problem hiding this comment.
Can be implemented in terms of single arg constructor
| this.file.close(); | ||
| } | ||
|
|
||
| private void dropUntil(String flag) { |
There was a problem hiding this comment.
High level of overlap between the Embl/Fasta/FastQ implementations. Refactor overlap into a shared dependency instead of duplicating code
There was a problem hiding this comment.
Refactored out into BufferedFileStreamReader class to provide help for performing buffered reads from files.
| @Override | ||
| public boolean hasNext() { | ||
| // Dump whitespace | ||
| dropUntil(x -> !Character.isWhitespace(x)); |
There was a problem hiding this comment.
Lambda's have a performance cost. Make dropUntilWhitespace() and dropUntilNotWhitespace() functions
| * @author John | ||
| * | ||
| */ | ||
| public class EmblSequenceStreamReader implements SequenceStreamReader { |
There was a problem hiding this comment.
Significant overlap with Fasta/Fastq implementations. Consider condensing to a single SequenceStreamReader
There was a problem hiding this comment.
Fixed - Using only a single SequenceStreamReader now
Move FASTQ test data to FastqData Move FASTA test data to FastaData Cleanup StringFileSreamReader tests Cleanup file names Cleanup SequenceStreamWriter tests
JMBattista
force-pushed
the
emblsupport
branch
from
f0d2b0b
to
247d954
Compare
| */ | ||
| public class EmblSequenceFactory implements SequenceFactory { | ||
|
|
||
| private EncodingScheme encodingSheme; |
There was a problem hiding this comment.
Should be encodingScheme
| /** | ||
| * Ignore characters in the buffer while they are whitespace | ||
| */ | ||
| public void dropWhileWhitespace() { |
There was a problem hiding this comment.
Why not use the predicate version? Efficiency?
| * Ignore characters in the buffer until they are whitespace | ||
| */ | ||
| public void dropUntilWhiteSpace() { | ||
| while (true) { |
There was a problem hiding this comment.
Like dropWhileWhitespace this could be implemented using the predicate function
There was a problem hiding this comment.
Yep. I do find it a little hard to differentiate dropUntil(x -> !Character.isWhitespace(x)) and dropUntil(x-> Character.isWhitespace(x)) when scanning the code as well. You can get around this using dropUntil(Character::isWhitespace) instead to differentiate it from the ! case.
I found that dropping whitespace, taking until whitespace, or dropping until whitespace were just very common actions so moving them into full functions makes it a bit easier and lambdas have a performance hit over direct reference.
| index++; | ||
| } | ||
|
|
||
| return sb.toString().trim(); |
There was a problem hiding this comment.
The fact that the resulting string is trimmed isn't evident from the comments on this function, although I imagine there's a reason for the result to be trimmed.
There was a problem hiding this comment.
It really shouldn't. It was apparently there due to windows having \r\n instead of just \n. I was able to remove the trim 👍
Remove large test files from repository
JMBattista
force-pushed
the
emblsupport
branch
from
ca2e920
to
1d33c7b
Compare
Add support for EMBL file format, cleanup test code for reading and writing file formats