commons-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Benedikt Ritter <brit...@apache.org>
Subject Re: [CSV] Creating CSVParser instances
Date Mon, 12 Aug 2013 19:13:09 GMT
2013/8/8 Gary Gregory <garydgregory@gmail.com>

> On Thu, Aug 8, 2013 at 10:27 AM, Benedikt Ritter <britter@apache.org>
> wrote:
>
> > 2013/8/8 Emmanuel Bourg <ebourg@apache.org>
> >
> > > Le 08/08/2013 15:40, Gary Gregory a écrit :
> > >
> > > > Sans type names:
> > > >
> > > > parse(File, CSVFormat)
> > > > parse(String, Charset, ClassLoader, CSVFormat)
> > > > parse(String, Charset, CSVFormat)
> > > > [parse(String)]
> > > > parse(String, CSVFormat)
> > > > parse(URL, Charset, CSVFormat)
> > >
> > > That looks better. I would remove the methods for a classpath resource,
> > > that's a less common case. That would make:
> > >
> > > parse(File, CSVFormat)
> > > parse(String, CSVFormat)
> > > parse(URL, Charset, CSVFormat)
> > >
> > > And you probably want a charset for the File too.
> > >
> > >
> > > > [I'd probably remove parse(String) so that all APIs take a
> CSVFormat.]
> > >
> > > +1.
> > >
> > > And at this point you realize they could belong to CSVFormat, because
> > > they all need one to operate.
> > >
> > >     format.parse(file):
> > >
> >
> > A format can parse something... That sounds strange to me.
> >
>
> Same here, it sounds strange that a format does anything like parsing. A
> parser parses, that's obvious. When I leave [csv] alone for a while and get
> back to it, the parser is always where I go look for an API to get started.
> It's just weird to start with a format IMO. I would be OK with leaving the
> format parser API there I suppose, but I do not think the format should be
> the kitchen sink for all other input sources. Well, you can try to convince
> me of course ;)
>

Okay, so do we want to let parse(Reader) method as a convenience in
CSVFormat?
The CSVParser will serve as the main entry point to the API.

What about the parse resource methods in CSVParser? Emmanuel has expressed
feelings against this addition. I personally think reading from the class
path should be done in client code. But since Gary seems to have a use case
for this and I cannot really judge how common it is to read csv data from
the class path I could live with this.

What I really don't like is:
   public static CSVParser parse(String, CSVFormat)
   public static CSVParser parse(String, CharSet, CSVFormat)

They look nearly the same yet they do completely different things... IMHO
this cannot stay this way.

WDYT?

Benedikt


>
> Gary
>
>
>
> > Let's rename it to
> >
> >    format.createParser(file)
> >
> > I'm +1 for having only one place to create parsers.
> > And having less parameters is (in most cases) better.
> >
> >
> > >
> > > instead of:
> > >
> > >     CSVParser.parse(file, format);
> > >
> > >
> > > Emmanuel Bourg
> > >
> > >
> > > ---------------------------------------------------------------------
> > > To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> > > For additional commands, e-mail: dev-help@commons.apache.org
> > >
> > >
> >
> >
> > --
> > 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

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