gbidsilva commented on code in PR #347:
URL: https://github.com/apache/commons-csv/pull/347#discussion_r1314921125
##########
src/main/java/org/apache/commons/csv/CSVParser.java:
##########
@@ -765,7 +765,13 @@ CSVRecord nextRecord() throws IOException {
final long startCharPosition = lexer.getCharacterPosition() +
this.characterOffset;
do {
this.reusableToken.reset();
- this.lexer.nextToken(this.reusableToken);
+ try {
+ this.lexer.nextToken(this.reusableToken);
+ } catch (IOException ioe) {
+ String errorMessage = "Exception during parsing at line: "
+ + this.lexer.getCurrentLineNumber() + ", position: " +
this.lexer.getCharacterPosition();
Review Comment:
@garydgregory
I had a look at what you are suggesting. Below are my thoughts...
If we look at the internals of method `Token nextToken(final Token token)
throws IOException{}` in class `Lexer`, there are around 10 places where
`IOException` can be thrown.
Among them method calls to `parseEncapsulatedToken()`, `parseSimpleToken()`
and `reader.read()` have high possibility of throwing parsing related
`IOException`.
In that case we have following options.
1. handle each of method calls for customized IOException throwings with our
enhanced exception message.
2. wrap `nextToken()` in class `Lexer` entirely and throw `IOException` with
enhanced exception message.
3. keep the `this.lexer.nextToken(this.reusableToken)` wrapped with try
catch in class `CSVParser` and rethrow IOException with enhanced message - this
is what we have at the moment in PR.
IMO, keeping the try catch block in the current place we have at the moment
seems more simple rather than handling exception in multiple places inside
`Lexer` class.
Appreciate your opinion on this.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
