commons-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Gary Gregory <garydgreg...@gmail.com>
Subject Re: [CSV] Creating CSVParser instances
Date Thu, 08 Aug 2013 13:10:07 GMT
On Thu, Aug 8, 2013 at 4:24 AM, Benedikt Ritter <britter@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: garydgregory@gmail.com | ggregory@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

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message