I do not think reverting is right since the previous is obviously
bogus, to
me, that is. Throwing a RE is never useful, again IMO. We should just
evolve the code from here.
Would you like to outline choices?
1) NSEE
2) Custom
3) ISE
4) ?
Gary (will be offline most of today)
On Oct 28, 2016 5:36 AM, "Benedikt Ritter" <brit...@apache.org>
wrote:
Hello,
Benedikt Ritter <brit...@apache.org> schrieb am Mi., 26. Okt. 2016 um
21:38 Uhr:
Hello,
<ggreg...@apache.org> schrieb am Di., 25. Okt. 2016 um 22:59 Uhr:
Repository: commons-csv
Updated Branches:
refs/heads/master b2a50d4a3 -> 1c7a9910b
[CSV-201] Do not use RuntimeException in CSVParser.iterator().new
Iterator() {...}.getNextRecord(). Use an IllegalStateException
instead
(KISS for now, no need for a custom exception)
Project: http://git-wip-us.apache.org/repos/asf/commons-csv/repo
Commit:
http://git-wip-us.apache.org/repos/asf/commons-csv/commit/1c7a9910
Tree:
http://git-wip-us.apache.org/repos/asf/commons-csv/tree/1c7a9910
Diff:
http://git-wip-us.apache.org/repos/asf/commons-csv/diff/1c7a9910
Branch: refs/heads/master
Commit: 1c7a9910be857a4f33ed216701407806e1d20670
Parents: b2a50d4
Author: Gary Gregory <ggreg...@apache.org>
Authored: Tue Oct 25 13:59:32 2016 -0700
Committer: Gary Gregory <ggreg...@apache.org>
Committed: Tue Oct 25 13:59:32 2016 -0700
----------------------------------------------------------------------
src/changes/changes.xml | 1 +
src/main/java/org/apache/commons/csv/CSVParser.java | 16
++++++++++------
2 files changed, 11 insertions(+), 6 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/commons-csv/blob/
1c7a9910/src/changes/changes.xml
----------------------------------------------------------------------
diff --git a/src/changes/changes.xml b/src/changes/changes.xml
index d5d8c8a..e49265b 100644
--- a/src/changes/changes.xml
+++ b/src/changes/changes.xml
@@ -43,6 +43,7 @@
<action issue="CSV-193" type="fix" dev="ggregory"
due-to="Matthias
Wiehl">Fix incorrect method name 'withFirstRowAsHeader' in user
guide.</action>
<action issue="CSV-171" type="fix" dev="ggregory"
due-to="Gary
Gregory, Michael Graessle, Adrian Bridgett">Negative numeric values
in the
first column are always quoted in minimal mode.</action>
<action issue="CSV-187" type="update" dev="ggregory"
due-to="Gary
Gregory">Update platform requirement from Java 6 to 7.</action>
+ <action issue="CSV-201" type="update" dev="ggregory"
due-to="Benedikt Ritter, Gary Gregory">Do not use RuntimeException
in
CSVParser.iterator().new Iterator() {...}.getNextRecord()</action>
<action issue="CSV-189" type="add" dev="ggregory"
due-to="Peter
Holzwarth, Gary Gregory">CSVParser: Add factory method accepting
InputStream.</action>
<action issue="CSV-190" type="add" dev="ggregory"
due-to="Gary
Gregory">Add convenience API CSVFormat.print(File, Charset)</action>
<action issue="CSV-191" type="add" dev="ggregory"
due-to="Gary
Gregory">Add convenience API CSVFormat.print(Path, Charset)</action>
http://git-wip-us.apache.org/repos/asf/commons-csv/blob/
1c7a9910/src/main/java/org/apache/commons/csv/CSVParser.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/commons/csv/CSVParser.java
b/src/main/java/org/apache/commons/csv/CSVParser.java
index f5829cc..410e6a4 100644
--- a/src/main/java/org/apache/commons/csv/CSVParser.java
+++ b/src/main/java/org/apache/commons/csv/CSVParser.java
@@ -501,10 +501,14 @@ public final class CSVParser implements
Iterable<CSVRecord>, Closeable {
/**
* Returns an iterator on the records.
*
- * <p>IOExceptions occurring during the iteration are wrapped
in a
- * RuntimeException.
- * If the parser is closed a call to {@code next()} will throw
a
- * NoSuchElementException.</p>
+ * <p>
+ * An {@link IOException} caught during the iteration are
re-thrown
as an
+ * {@link IllegalStateException}.
+ * </p>
+ * <p>
+ * If the parser is closed a call to {@link Iterator#next()}
will
throw a
+ * {@link NoSuchElementException}.
+ * </p>
*/
@Override
public Iterator<CSVRecord> iterator() {
@@ -515,8 +519,8 @@ public final class CSVParser implements
Iterable<CSVRecord>, Closeable {
try {
return CSVParser.this.nextRecord();
} catch (final IOException e) {
- // TODO: This is not great, throw an ISE
instead?
- throw new RuntimeException(e);
+ throw new IllegalStateException(
+ e.getClass().getSimpleName() + "
reading next
record: " + e.toString(), e);
As discussed in CSV-201, I don't think this fix is complete. We
should
rather throw a custom RuntimeException to make it easy for calling
code to
react to exactly this problem.
I think this needs to be reverted until we have consensus about how
to
solve this issue.
Benedikt
}
}
