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. 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
