-
Notifications
You must be signed in to change notification settings - Fork 5
create universal reader and writer #209
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
our internal file read and writer
protected T next; | ||
|
||
protected List<String> headerLines = new ArrayList<>(); | ||
public RecordReader(final File file) throws IOException { this(file, DEFAULT_BUFFER_SIZE); } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
formatting
/** | ||
* Here, BufferedReader.close() calls InputStreamReader.close(), which API told us that it Closes the stream and releases any system resources associated with it. | ||
*/ | ||
public void close() throws IOException { bin.close(); } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
formatting
* This reader can maxmum take Integer.max lines of file header. Please make other header if bigger than this. | ||
* @return a list of header lines | ||
*/ | ||
public List<String> getHeader() { return headerLines; } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
formatting
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
does this pass checkstyle's checks?
if(headerPrefix == null) return nextLine; | ||
|
||
//reader header, hence file pointer to first line after header | ||
while ( headerPrefix != null && null != nextLine && nextLine.startsWith(headerPrefix+"") ) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
null guards have already been performed
try { | ||
next = nextLine == null? null : getRecord(nextLine); | ||
}catch(Exception e) { | ||
throw new IOException("error during retrive first record " + e.getMessage()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
spelling
} | ||
|
||
//some record cross multi lines, eg id\nseq\n, this method may call bin.readLine() inside | ||
public abstract T getRecord(String line) throws Exception; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could a more specific Exception by thrown here?
* @return the first line just after header | ||
* @throws IOException | ||
*/ | ||
public String readHeader(CharSequence headerPrefix ) throws IOException{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe change to more meaningful name ie. readHeaderAndReturnFirstNonHeaderLine
} | ||
|
||
return rec; | ||
} catch (Exception e) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should avoid catching Exception - be more specific
* This reader can maxmum take Integer.max lines of file header. Please make other header if bigger than this. | ||
* @return a list of header lines | ||
*/ | ||
public List<String> getHeader() { return headerLines; } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
does this pass checkstyle's checks?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Description
Our current qio project contains more than 20 different file readers and writers, most of them are similar. Some of them are hardly maintained due to old or deprecated file format. Here a universal file reader and writer is created, which can cover most file format, and also can be used as a base class for special and the complicated file format, such as VCF, FASTA, GFF, Illumina, etc.
Fixes #196 #197 #198
Type of change
Please delete options that are not relevant.
How Has This Been Tested?
A unit test class is added. At moment these three class is not yet used in any projects, it will be further tested after replace current old other reader and writer
Checklist: