You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@commons.apache.org by Benedikt Ritter <br...@apache.org> on 2013/09/13 13:22:34 UTC

Re: [csv] type name consistency

We never finished the discussion about Quote. I agree with Gary that it's
weird to have Quote as the only class that doesn't have the "CSV" prefix.

I'll change this if nobody objects.

Benedikt


2013/8/7 Gary Gregory <ga...@gmail.com>

> On Wed, Aug 7, 2013 at 10:39 AM, sebb <se...@gmail.com> wrote:
>
> > On 7 August 2013 14:15, Gary Gregory <ga...@gmail.com> wrote:
> > > On Wed, Aug 7, 2013 at 2:45 AM, Benedikt Ritter <br...@apache.org>
> > wrote:
> > >
> > >> 2013/8/7 Gary Gregory <ga...@gmail.com>
> > >>
> > >> > I think we can be done unless other opinions come in...
> > >> >
> > >>
> > >> What about Quote? Do we want to move it to CSVFormat? Makes sense
> > because
> > >> it is really part of a CSVFormat. OTOH CSVFormat.Quote.ALL is more
> > verbose
> > >> then simply Quote.ALL
> > >>
> > >
> > > If we consider an inner class, I think it should be one of:
> > >
> > > CSVFormat.Quote.ALL
> > > CSVQuote.ALL
> > >
> > > It's too weird to leave Quote as the only public name that is not
> > prefixed.
> >
> > CSVLexer is package private.
> > Is this weird?
> > Should it be renamed to Lexer?
> >
>
> I'm OK with that.
>
> Gary
>
> >
> > > Gary
> > >
> > >
> > >>
> > >> >
> > >> > G
> > >> >
> > >> >
> > >> > On Tue, Aug 6, 2013 at 6:57 PM, sebb <se...@gmail.com> wrote:
> > >> >
> > >> > > On 6 August 2013 17:47, Gary Gregory <ga...@gmail.com>
> > wrote:
> > >> > > > On Tue, Aug 6, 2013 at 11:27 AM, Benedikt Ritter <
> > britter@apache.org
> > >> >
> > >> > > wrote:
> > >> > > >
> > >> > > >> 2013/8/6 sebb <se...@gmail.com>
> > >> > > >>
> > >> > > >> > On 6 August 2013 15:48, Gary Gregory <garydgregory@gmail.com
> >
> > >> > wrote:
> > >> > > >> > > The class names in [csv] are (pp) means package private,
> > others
> > >> > are
> > >> > > >> > public
> > >> > > >> > >
> > >> > > >> > > org.apache.commons.csv.Constants (pp)
> > >> > > >> > > org.apache.commons.csv.CSVFormat
> > >> > > >> > > org.apache.commons.csv.CSVLexer
> > >> > > >> >
> > >> > > >> > That is also (pp)
> > >> > > >> >
> > >> > > >> > > org.apache.commons.csv.CSVParser
> > >> > > >> > > org.apache.commons.csv.CSVPrinter
> > >> > > >> > > org.apache.commons.csv.CSVRecord
> > >> > > >> > > org.apache.commons.csv.ExtendedBufferedReader (pp)
> > >> > > >> > > org.apache.commons.csv.Lexer (pp)
> > >> > > >> > > org.apache.commons.csv.Quote
> > >> > > >> > > org.apache.commons.csv.Token (pp)
> > >> > > >> > >
> > >> > > >> > > So all of the _public_ types except Quote are prefixed with
> > >> "CSV".
> > >> > > >> > >
> > >> > > >> > > - Should all types be prefixed with "CSV"?
> > >> > > >> > > - Should Quote be renamed to CSVQuote?
> > >> > > >> > >
> > >> > > >> > > - I personally, prefer the prefix "Csv", but I might be in
> > the
> > >> > > minority
> > >> > > >> > > here.
> > >> > > >> >
> > >> > > >> > Could also rename Lexer as AbstractLexer and then drop all
> the
> > >> > CSV/Csv
> > >> > > >> > prefixes.
> > >> > > >> >
> > >> > > >> > And/or possibly merge Lexer and CSVLexer.
> > >> > > >> >
> > >> > > >>
> > >> > > >> big +1 from me. There is no need to have a package private
> > abstract
> > >> > base
> > >> > > >> class if we only have one package private implementation.
> > >> > > >>
> > >> > > >
> > >> > > > Sebb just did this work, thank you Sebb!
> > >> > >
> > >> > > I only merged Lexer into CSVLexer.
> > >> > >
> > >> > > The naming issue (if it is one) still remains.
> > >> > >
> > >> > > > Gary
> > >> > > >
> > >> > > >
> > >> > > >>
> > >> > > >>
> > >> > > >> > I think they were separated to make testing alternate
> > >> > implementations
> > >> > > >> > easier.
> > >> > > >> > But it's probably better to just copy all the relevant
> classes
> > if
> > >> we
> > >> > > >> > need to revisit the benchmarks.
> > >> > > >> >
> > >> > > >> > > Gary
> > >> > > >> > >
> > >> > > >> > > --
> > >> > > >> > > 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
> > >> > > >> >
> > >> > > >> >
> > >> >
> ---------------------------------------------------------------------
> > >> > > >> > To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> > >> > > >> > For additional commands, e-mail: dev-help@commons.apache.org
> > >> > > >> >
> > >> > > >> >
> > >> > > >>
> > >> > > >>
> > >> > > >> --
> > >> > > >> 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
> > >> > >
> > >> > >
> > ---------------------------------------------------------------------
> > >> > > To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> > >> > > For additional commands, e-mail: dev-help@commons.apache.org
> > >> > >
> > >> > >
> > >> >
> > >> >
> > >> > --
> > >> > 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
> > >> >
> > >>
> > >>
> > >>
> > >> --
> > >> 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
> >
> > ---------------------------------------------------------------------
> > To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> > For additional commands, e-mail: dev-help@commons.apache.org
> >
> >
>
>
> --
> 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
>



-- 
http://people.apache.org/~britter/
http://www.systemoutprintln.de/
http://twitter.com/BenediktRitter
http://github.com/britter

Re: [csv] type name consistency

Posted by Benedikt Ritter <br...@apache.org>.
2013/9/14 Emmanuel Bourg <eb...@apache.org>

> Le 14/09/2013 11:27, sebb a écrit :
>
> > There is one other inconsistency that might be worth addressing: all
> > the classes are final apart from CSVFormat.
> > Should it be final? If not, the reason needs documenting as it is
> > different from the rest of the code.
>
> +1 for making CSVFormat final.
>

Done in revision 1523210.


>
> Emmanuel Bourg
>
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> For additional commands, e-mail: dev-help@commons.apache.org
>
>


-- 
http://people.apache.org/~britter/
http://www.systemoutprintln.de/
http://twitter.com/BenediktRitter
http://github.com/britter

Re: [csv] type name consistency

Posted by Emmanuel Bourg <eb...@apache.org>.
Le 14/09/2013 11:27, sebb a écrit :

> There is one other inconsistency that might be worth addressing: all
> the classes are final apart from CSVFormat.
> Should it be final? If not, the reason needs documenting as it is
> different from the rest of the code.

+1 for making CSVFormat final.

Emmanuel Bourg



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org

Re: [csv] type name consistency

Posted by sebb <se...@gmail.com>.
On 14 September 2013 09:20, Emmanuel Bourg <eb...@apache.org> wrote:
> Le 13/09/2013 13:22, Benedikt Ritter a écrit :
>
>> I'll change this if nobody objects.
>
> I'm still not convinced by the renaming of Quote as a matter of
> consistency. Does it mean that any new public class added to the package
> in the future will have to be prefixed by "CSV" as well? That seems a
> bit weird to me. It should be prefixed when it really makes sense.
>
> 'Quote' is short and not likely to conflict with other classes (unlike
> Parser and Format). I would keep it as is.

+1

There is one other inconsistency that might be worth addressing: all
the classes are final apart from CSVFormat.
Should it be final? If not, the reason needs documenting as it is
different from the rest of the code.

> Emmanuel Bourg
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> For additional commands, e-mail: dev-help@commons.apache.org
>

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org

Re: [csv] type name consistency

Posted by Emmanuel Bourg <eb...@apache.org>.
Le 13/09/2013 13:22, Benedikt Ritter a écrit :

> I'll change this if nobody objects.

I'm still not convinced by the renaming of Quote as a matter of
consistency. Does it mean that any new public class added to the package
in the future will have to be prefixed by "CSV" as well? That seems a
bit weird to me. It should be prefixed when it really makes sense.

'Quote' is short and not likely to conflict with other classes (unlike
Parser and Format). I would keep it as is.

Emmanuel Bourg


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org