Hello Gary,
please see my comments in CSV-201.
Thank you,
Benedikt
Gary Gregory <garydgreg...@gmail.com> schrieb am Fr. 28. Okt. 2016 um 17:05:
> 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
> >
> >
> > }
> > }
> >
> >
> >
>
