On Wed, Aug 14, 2013 at 2:09 PM, Benedikt Ritter <brit...@apache.org> wrote:
> Done in revision 1513994. > > Every factory method an the constructor now take a CSVFormat. How about > using CSVFormat.DEFAULT is null is passed as CSVFormat? > Seems reasonable. Gary > > Benedikt > > > 2013/8/14 Matt Benson <gudnabr...@gmail.com> > > > To stir the pot, one might even make a case for using Reader only ;-) . > > > > Matt > > On Aug 14, 2013 8:03 AM, "Gary Gregory" <garydgreg...@gmail.com> wrote: > > > > > On Wed, Aug 14, 2013 at 2:44 AM, 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? > > > > > > > > > > Go for it. > > > > > > Gary > > > > > > > > > > > > > > > > > > > > > > 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 > > > > > > > > > > > > > > > > -- > > > 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
