On Thu, Aug 8, 2013 at 9:10 AM, Gary Gregory <garydgreg...@gmail.com> wrote:
> On Thu, Aug 8, 2013 at 4:24 AM, Benedikt Ritter <brit...@apache.org>wrote: > >> Hi, >> >> we currently have several static factory methods in CSVParser: >> >> - public static CSVParser parseFile(File file, final CSVFormat format) >> throws IOException >> - public static CSVParser parseResource(String resource, Charset charset, >> ClassLoader classLoader, >> final CSVFormat format) throws IOException >> - public static CSVParser parseResource(String resource, Charset charset, >> final CSVFormat format) throws IOException >> - public static CSVParser parseString(String string) throws IOException >> - public static CSVParser parseString(String string, final CSVFormat >> format) throws IOException >> - public static CSVParser parseURL(URL url, Charset charset, final >> CSVFormat format) throws IOException >> >> and one instance factory method in CSVFormat: >> >> - public CSVParser parse(final Reader in) throws IOException >> >> One can also create a parser using the public constructors defined in >> CSVParser: >> >> - public CSVParser(final Reader input) throws IOException >> - public CSVParser(final Reader reader, final CSVFormat format) throws >> IOException >> >> I'm wondering: >> >> 1. do we need all this different ways to create CSVParsers? For example it >> may be confusing to have parse(Reader) in CSVFormat which is pretty much >> the same as CSVParser(Reader, CSVFormat) just the other way around. >> >> 2. all the factory methods are named "parseXXX" but they don't actually >> parse anything. They just create an object that is capable of parsing CSV >> content. Should the factory methods be renamed? >> > > Here is how I see it: > > The one instance factory method in CSVFormat is a convenience that I do > not find useful, it just makes for a nice demo _in theory_. In my > experience, I always create utility methods to create Readers from > different sources. I would be OK removing this method in favor of > centralized construction in CSVParser or a new CSVParseFactory. > Well, there is no need for a CSVParseFactory of course, the CSVParse ctors can be private and the same feature (Reader in) can be in a factory method. This gives us the freedom to change the parser ctors later. Gary > > If we had a CSVParseFactory, we could remove parsing in CSVFormat and make > the CSVParser ctors package private. > > The naming pattern for factory method could also be "createParser" or > "create[Type]Parser". The only time the type name is really needed IMO is > to distinguish a resource string, from CSV content. I am assuming that a > file would be passed in as a File, instead of a file name. > > Gary > > >> Benedikt >> >> >> -- >> 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 > -- 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
