commons-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Gary Gregory <garydgreg...@gmail.com>
Subject Re: svn commit: r1578191 - /commons/proper/csv/trunk/src/main/java/org/apache/commons/csv/CSVParser.java
Date Mon, 17 Mar 2014 18:21:02 GMT
On Mon, Mar 17, 2014 at 1:48 PM, sebb <sebbaz@gmail.com> wrote:

> On 17 March 2014 17:26, Benedikt Ritter <britter@apache.org> wrote:
> > 2014-03-17 18:15 GMT+01:00 Gary Gregory <garydgregory@gmail.com>:
> >
> >> On Mon, Mar 17, 2014 at 12:35 PM, Benedikt Ritter <britter@apache.org
> >> >wrote:
> >>
> >> > Hi all,
> >> >
> >> > is it sensible to fall back to UTF-8? Looks like an opportunity for
> bugs
> >> to
> >> > sneak in. I'd rather have:
> >> >
> >> >  public static CSVParser parse(final URL url, final Charset charset,
> >> final
> >> > CSVFormat format)
> >> >
> >> > where none of the params must be null, and:
> >> >
> >> >  public static CSVParser parse(final URL url, final CSVFormat format)
> >> >
> >> > which uses UTF-8. This would be more explicit IMHO.
> >> >
> >> > WDYT?
> >> >
> >>
> >> IIRC the JRE uses the platform encoding if a charset is null for some
> APIs,
> >> so we could do that as well.
> >>
> >
> > Which is equally bad, IMHO. What is your opinion WRT default values?
> > If you have a bug in your app that for what ever reason passes null
> instead
> > of the charset you wanted it to pass, you're in trouble. That's why I
> think
> > providing two distinct methods is the better option here.
>
> I'm inclined to agree.
> Defaulting null may make sense where there is a clear "best" choice
> for the parameter.
> But that is not really the case here for a charset - there is no
> "correct" default for CSV files.
>

OK, then we are back to null -> exception. Check?

Gary


>
> >
> >>
> >> Gary
> >>
> >>
> >> > Benedikt
> >> >
> >> >
> >> > 2014-03-17 1:50 GMT+01:00 <ggregory@apache.org>:
> >> >
> >> > > Author: ggregory
> >> > > Date: Mon Mar 17 00:50:55 2014
> >> > > New Revision: 1578191
> >> > >
> >> > > URL: http://svn.apache.org/r1578191
> >> > > Log:
> >> > > The charset can be null and will default to UTF-8.
> >> > >
> >> > > Modified:
> >> > >
> >> > >
> >> >
> >>
> commons/proper/csv/trunk/src/main/java/org/apache/commons/csv/CSVParser.java
> >> > >
> >> > > Modified:
> >> > >
> >> >
> >>
> commons/proper/csv/trunk/src/main/java/org/apache/commons/csv/CSVParser.java
> >> > > URL:
> >> > >
> >> >
> >>
> http://svn.apache.org/viewvc/commons/proper/csv/trunk/src/main/java/org/apache/commons/csv/CSVParser.java?rev=1578191&r1=1578190&r2=1578191&view=diff
> >> > >
> >> > >
> >> >
> >>
> ==============================================================================
> >> > > ---
> >> > >
> >> >
> >>
> commons/proper/csv/trunk/src/main/java/org/apache/commons/csv/CSVParser.java
> >> > > (original)
> >> > > +++
> >> > >
> >> >
> >>
> commons/proper/csv/trunk/src/main/java/org/apache/commons/csv/CSVParser.java
> >> > > Mon Mar 17 00:50:55 2014
> >> > > @@ -183,7 +183,7 @@ public final class CSVParser implements
> >> > >       * @param url
> >> > >       *            a URL. Must not be null.
> >> > >       * @param charset
> >> > > -     *            the charset for the resource. Must not be null.
> >> > > +     *            the charset for the resource. If {@code null},
> use
> >> > > {@code UTF-8}.
> >> > >       * @param format
> >> > >       *            the CSVFormat used for CSV parsing. Must not be
> >> null.
> >> > >       * @return a new parser
> >> > > @@ -194,7 +194,6 @@ public final class CSVParser implements
> >> > >       */
> >> > >      public static CSVParser parse(final URL url, final Charset
> >> charset,
> >> > > final CSVFormat format) throws IOException {
> >> > >          Assertions.notNull(url, "url");
> >> > > -        Assertions.notNull(charset, "charset");
> >> > >          Assertions.notNull(format, "format");
> >> > >
> >> > >          return new CSVParser(new
> InputStreamReader(url.openStream(),
> >> > >
> >> > >
> >> > >
> >> >
> >> >
> >> > --
> >> > http://people.apache.org/~britter/
> >> > http://www.systemoutprintln.de/
> >> > http://twitter.com/BenediktRitter
> >> > http://github.com/britter
> >> >
> >>
> >>
> >>
> >> --
> >> E-Mail: garydgregory@gmail.com | ggregory@apache.org
> >> Java Persistence with Hibernate, Second Edition<
> >> http://www.manning.com/bauer3/>
> >> JUnit in Action, Second Edition <http://www.manning.com/tahchiev/>
> >> Spring Batch in Action <http://www.manning.com/templier/>
> >> Blog: http://garygregory.wordpress.com
> >> Home: http://garygregory.com/
> >> Tweet! http://twitter.com/GaryGregory
> >>
> >
> >
> >
> > --
> > http://people.apache.org/~britter/
> > http://www.systemoutprintln.de/
> > http://twitter.com/BenediktRitter
> > http://github.com/britter
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> For additional commands, e-mail: dev-help@commons.apache.org
>
>


-- 
E-Mail: garydgregory@gmail.com | ggregory@apache.org
Java Persistence with Hibernate, Second Edition<http://www.manning.com/bauer3/>
JUnit in Action, Second Edition <http://www.manning.com/tahchiev/>
Spring Batch in Action <http://www.manning.com/templier/>
Blog: http://garygregory.wordpress.com
Home: http://garygregory.com/
Tweet! http://twitter.com/GaryGregory

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