Minor Improvements#127
Conversation
arturobernalg
commented
Dec 18, 2020
- Add final
- Unnecessary semicolon ''
- Use StandardCharsets
- Fix javadoc
| * Tests reusing a parser to process new string records one at a time as they are being discovered. See [CSV-110]. | ||
| * | ||
| * @throws IOException | ||
| * @throws Exception Any exception can be thrown |
| * Verifies the consistency of the parameters and throws an IllegalArgumentException if necessary. | ||
| * | ||
| * @throws IllegalArgumentException | ||
| * thrown if the delimiter is a scape character |
There was a problem hiding this comment.
changed. What do you thing?=
| * Tests reusing a parser to process new string records one at a time as they are being discovered. See [CSV-110]. | ||
| * | ||
| * @throws IOException | ||
| * @throws IOException If something goes wrong |
There was a problem hiding this comment.
Might as well use the "standard" comment here
There was a problem hiding this comment.
Sorry Garry, but What is the standard comment?
There was a problem hiding this comment.
@arturobernalg
The JRE uses "if an I/O error occurs." but I prefer "Thrown when an I/O error occurs."
There was a problem hiding this comment.
ok, got it. changed
|
|
||
| private static int num = 0; // number of elapsed times recorded | ||
| private static long[] elapsedTimes = new long[max]; | ||
| private static final long[] elapsedTimes = new long[max]; |
There was a problem hiding this comment.
Constants are usually in uppercase.
| // Now parse the file and compare against the expected results | ||
| final URL resource = ClassLoader.getSystemResource("org/apache/commons/csv/CSVFileParser/" + split[0]); | ||
| try (final CSVParser parser = CSVParser.parse(resource, Charset.forName("UTF-8"), format)) { | ||
| try (final CSVParser parser = CSVParser.parse(resource, StandardCharsets.UTF_8, format)) { |
| * Verifies the consistency of the parameters and throws an IllegalArgumentException if necessary. | ||
| * | ||
| * @throws IllegalArgumentException | ||
| * @throws IllegalArgumentException If something goes wrong |
There was a problem hiding this comment.
Hi @arturobernalg
This change does not help anyone, and it's confusing when the line above tells you when the exception would be thrown. I've updated git master for this Javadoc. Please rebase on master. TY.
There was a problem hiding this comment.
OK. Got it. Changed
| * Tests an exported Excel worksheet with a header row and rows that have more columns than the headers | ||
| * | ||
| * @throws Exception | ||
| * @throws Exception Any exception can be thrown |
There was a problem hiding this comment.
Not helpful. Tests like this one can throw Exception as a convention instead of throwing a potentially long list of exceptions which are not useful and just clutter up the method signatures. So either we should not have a Javadoc tag or if we have one, it should carry proper and useful documentation. In this case, I think that there is nothing interesting to say.
There was a problem hiding this comment.
OK, got it. Reverted
* Add final * Unnecessary semicolon '' * Use StandardCharsets * Fix javadoc