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. Would it be okay for you if we remove those methods? 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. 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
