commons-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Benedikt Ritter <brit...@apache.org>
Subject Re: commons-csv git commit: [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)
Date Fri, 28 Oct 2016 12:35:48 GMT
Hello,

Benedikt Ritter <britter@apache.org> schrieb am Mi., 26. Okt. 2016 um
21:38 Uhr:

> Hello,
>
> <ggregory@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 <ggregory@apache.org>
> Authored: Tue Oct 25 13:59:32 2016 -0700
> Committer: Gary Gregory <ggregory@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
>
>
>                  }
>              }
>
>
>

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message