On 14 August 2013 07:44, Benedikt Ritter <brit...@apache.org> wrote: > 2013/8/12 Gary Gregory <garydgreg...@gmail.com> > >> On Mon, Aug 12, 2013 at 3:13 PM, Benedikt Ritter <brit...@apache.org> >> wrote: >> >> > 2013/8/8 Gary Gregory <garydgreg...@gmail.com> >> > >> > > On Thu, Aug 8, 2013 at 10:27 AM, Benedikt Ritter <brit...@apache.org> >> > > wrote: >> > > >> > > > 2013/8/8 Emmanuel Bourg <ebo...@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? >> > >> >> Then it is back to "parseResource" or removing it altogether. Either way, >> I'll live with ;) >> > > Hey Gary, > > I know that reading from classpath is a use case for your app. > But to be honest I feel that this doesn't fit into the parser. > Fiddling around with the classpath should be done in client code.
Agreed. > Would it be okay for you if we remove those methods? I had not noticed the classpath methods otherwise I would have objected earlier to including them. > that would leave us with: > > public static CSVParser parse(File file, final CSVFormat format) throws > IOException > public static CSVParser parse(String string, final CSVFormat format) throws > IOException > public static CSVParser parse(URL url, Charset charset, final CSVFormat > format) throws IOException > > public CSVParser(final Reader input) throws IOException > public CSVParser(final Reader reader, final CSVFormat format) throws > IOException > > Looks clear to me. Only minor nit is that we only have parser methods that > take a CSVFormat but we have a constructor with and without. We can > probably remove the one that only takes a reader. Agreed. It's not necessary. > Benedikt > > >> >> Gary >> >> >> > >> > 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-unsubscr...@commons.apache.org >> > > > > For additional commands, e-mail: dev-h...@commons.apache.org >> > > > > >> > > > > >> > > > >> > > > >> > > > -- >> > > > http://people.apache.org/~britter/ >> > > > http://www.systemoutprintln.de/ >> > > > http://twitter.com/BenediktRitter >> > > > http://github.com/britter >> > > > >> > > >> > > >> > > >> > > -- >> > > E-Mail: garydgreg...@gmail.com | ggreg...@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 >> > >> >> >> >> -- >> E-Mail: garydgreg...@gmail.com | ggreg...@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-unsubscr...@commons.apache.org For additional commands, e-mail: dev-h...@commons.apache.org
