You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@commons.apache.org by Benedikt Ritter <br...@apache.org> on 2014/03/17 17:35:41 UTC
Re: svn commit: r1578191 - /commons/proper/csv/trunk/src/main/java/org/apache/commons/csv/CSVParser.java
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?
Benedikt
2014-03-17 1:50 GMT+01:00 <gg...@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
Re: svn commit: r1578191 - /commons/proper/csv/trunk/src/main/java/org/apache/commons/csv/CSVParser.java
Posted by Adrian Crum <ad...@sandglass-software.com>.
Design philosophies like this should be documented in the project - that
way there is no need to hash out the correct approach for every line of
code.
Everyone has different design philosophies, and in some cases there is
no "right" or "wrong" philosophy. [In this thread, two fail-slow designs
that provide default values, and one fail-fast design that throws an
exception.] So, it helps if a project can state clearly its fundamental
design philosophy. Some users might disagree with it, but at least it
makes code maintenance and new development easier.
Adrian Crum
Sandglass Software
www.sandglass-software.com
On 3/17/2014 11:42 AM, Gary Gregory wrote:
> I re-implemented the simplest behavior (back to what we had before): null
> -> IAE.
>
> Gary
>
>
> On Mon, Mar 17, 2014 at 2:31 PM, Emmanuel Bourg <eb...@apache.org> wrote:
>
>> Le 17/03/2014 19:21, Gary Gregory a écrit :
>>
>>> OK, then we are back to null -> exception. Check?
>>
>> I'd say default to the system encoding, like most methods of this kind.
>>
>> Emmanuel Bourg
>>
>>
>>
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
>> For additional commands, e-mail: dev-help@commons.apache.org
>>
>>
>
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
Re: svn commit: r1578191 - /commons/proper/csv/trunk/src/main/java/org/apache/commons/csv/CSVParser.java
Posted by Gary Gregory <ga...@gmail.com>.
I re-implemented the simplest behavior (back to what we had before): null
-> IAE.
Gary
On Mon, Mar 17, 2014 at 2:31 PM, Emmanuel Bourg <eb...@apache.org> wrote:
> Le 17/03/2014 19:21, Gary Gregory a écrit :
>
> > OK, then we are back to null -> exception. Check?
>
> I'd say default to the system encoding, like most methods of this kind.
>
> Emmanuel Bourg
>
>
>
> ---------------------------------------------------------------------
> 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
Re: svn commit: r1578191 - /commons/proper/csv/trunk/src/main/java/org/apache/commons/csv/CSVParser.java
Posted by Emmanuel Bourg <eb...@apache.org>.
Le 17/03/2014 19:21, Gary Gregory a écrit :
> OK, then we are back to null -> exception. Check?
I'd say default to the system encoding, like most methods of this kind.
Emmanuel Bourg
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org
Re: svn commit: r1578191 - /commons/proper/csv/trunk/src/main/java/org/apache/commons/csv/CSVParser.java
Posted by Gary Gregory <ga...@gmail.com>.
On Mon, Mar 17, 2014 at 1:48 PM, sebb <se...@gmail.com> wrote:
> On 17 March 2014 17:26, Benedikt Ritter <br...@apache.org> wrote:
> > 2014-03-17 18:15 GMT+01:00 Gary Gregory <ga...@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 <gg...@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
Re: svn commit: r1578191 - /commons/proper/csv/trunk/src/main/java/org/apache/commons/csv/CSVParser.java
Posted by sebb <se...@gmail.com>.
On 17 March 2014 17:26, Benedikt Ritter <br...@apache.org> wrote:
> 2014-03-17 18:15 GMT+01:00 Gary Gregory <ga...@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.
>
>>
>> Gary
>>
>>
>> > Benedikt
>> >
>> >
>> > 2014-03-17 1:50 GMT+01:00 <gg...@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
Re: svn commit: r1578191 - /commons/proper/csv/trunk/src/main/java/org/apache/commons/csv/CSVParser.java
Posted by Benedikt Ritter <br...@apache.org>.
2014-03-17 18:15 GMT+01:00 Gary Gregory <ga...@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.
>
> Gary
>
>
> > Benedikt
> >
> >
> > 2014-03-17 1:50 GMT+01:00 <gg...@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
Re: svn commit: r1578191 - /commons/proper/csv/trunk/src/main/java/org/apache/commons/csv/CSVParser.java
Posted by Gary Gregory <ga...@gmail.com>.
On Mon, Mar 17, 2014 at 12:35 PM, Benedikt Ritter <br...@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.
Gary
> Benedikt
>
>
> 2014-03-17 1:50 GMT+01:00 <gg...@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