commons-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Benedikt Ritter <brit...@apache.org>
Subject Re: [CSV] Naming pattern of getters and setters in CSVFormat (was: Re: [VOTE] Release Commons CSV 1.0 based on RC1)
Date Mon, 21 Jul 2014 18:45:45 GMT
Hey Gary,

I like what you did with the isEscaping, isHandlingNull etc. The API looks
much cleaner to me now.

What I don't understand is, why you added the type postfix to some of the
getters. If we have getCommentStartCharacter, why don't we have
getRecordSeparatorString ?

We're very close...

Benedikt


2014-07-21 19:18 GMT+02:00 Gary Gregory <garydgregory@gmail.com>:

> On Mon, Jul 21, 2014 at 12:35 PM, Benedikt Ritter <britter@apache.org>
> wrote:
>
> > Please take another look at CSV in rev. 1612344. I have:
> >
> > - renamed the ignoreEmptyHeaders property to allowMissingColumnNames
> > - prefixed everything that is a pure getter with "get"
> > - renamed Quote/QuotePolicy consistently to QuoteMode
> >
>
> Good changes!
>
>
> > Is this the final API we can agree upon?
> >
>
> Not quite, these names are still bad IMO:
>
> org.apache.commons.csv.CSVFormat.isHandlingNull()
>
> Terrible name, isNullStringSet() is simpler and expresses what the code
> does. FWIW, It is also the same kind of name JAXB generates for this code
> pattern.
>
> org.apache.commons.csv.CSVFormat.isQuoting()
>
> isFoo should return a foo ivar. This is not the case here.
>
> org.apache.commons.csv.CSVFormat.isCommentingEnabled() return a computed,
> so the name is good.
>
> Same for org.apache.commons.csv.CSVFormat.isEscaping() but why does one
> have the "Enabled" postfix and not the other? The names should be
> consistent, I say remove "Enabled".
>
> So to summarize, looking at all of:
>
> isCommentingEnabled()
> isEscaping()
> isHandlingNull()
> isQuoting()
>
> they all do the same thing: test an ivar for null.
>
> So the simple thing to do is to make sure we have a good ivar name and use
> the JAXB inspired pattern: ivar foo has a test method called isFooSet()
>
> commentStart : Character
> escape : Character
> quoteChar : Character
>
> First, why is one name postFixed with Char and not the others? That should
> be consistent. And why not use the full name "Character" in the postfix?
> Let's do that and match the method names.
>
> This also gives us room for String versions later, getCommentStartString()
> for example, without breaking BC.
>
> For the with* methods, having the Character postfix does not make sense,
> since the type of the argument is in the signature. So
> withQuoteChar(char|Character) should be come withQuote(char|Character).
>
> Now, that's better :-)
>
> My only question now is should "char delimiter" be "char delimiterChar"?
>
> See revision 1612352.
>
> Gary
>
>
> >
> > br,
> > Benedikt
> >
> >
> > 2014-07-20 14:04 GMT+02:00 Gary Gregory <garydgregory@gmail.com>:
> >
> > > I like using all "get" methods and no "is" methods. It is simpler and
> > > makes the getters easier to access as a group with code completion IMO.
> > The
> > > with methods do not behave like Java bean method so I do not thing we
> > need
> > > to worry about that. Unless we want to register immutability...
> > >
> > > Gary
> > >
> > > <div>-------- Original message --------</div><div>From: Benedikt
> Ritter <
> > > britter@apache.org> </div><div>Date:07/20/2014  04:02  (GMT-05:00)
> > > </div><div>To: Commons Developers List <dev@commons.apache.org>
> > > </div><div>Subject: Re: [CSV] Naming pattern of getters and setters
in
> > > CSVFormat (was: Re: [VOTE] Release Commons CSV 1.0 based on RC1)
> > </div><div>
> > > </div>using "get" for methods that return booleans is very uncommon
> > imho...
> > >
> > > how about leaving all the gramme stuff out and use:
> > >
> > > void withSkipEmptyHeaders(boolean)
> > > boolean isSkipEmptyHeaders
> > >
> > > that would
> > > - restore symmetry between getter and setter
> > > - almost follow JavaBean conventions (except for the "with")
> > >
> > > br,
> > > Benedikt
> > >
> > >
> > > 2014-07-20 8:00 GMT+02:00 Dipanjan Laha <dipanjan21@gmail.com>:
> > >
> > > > Although i am not familiar with CSV's codebase, imho "get" is more
> > > straight
> > > > forward, so +1 to Gary's suggestion.
> > > >
> > > > On Saturday, 19 July 2014, Gary Gregory <garydgregory@gmail.com>
> > wrote:
> > > >
> > > > > On Sat, Jul 19, 2014 at 12:14 PM, Emmanuel Bourg <
> ebourg@apache.org
> > > > > <javascript:;>> wrote:
> > > > >
> > > > > > Le 19/07/2014 13:48, Gary Gregory a écrit :
> > > > > >
> > > > > > > Can we go back to use "get"?
> > > > > >
> > > > > > We are running in circles Gary, Benedikt and I, if others could
> > weigh
> > > > in
> > > > > > that would help.
> > > > > >
> > > > >
> > > > > Circles, back and forth, to and fro, call it what you will. IMO
> this
> > is
> > > > the
> > > > > nature of the kind of development we do. Decentralized, no water
> > > cooler,
> > > > no
> > > > > white board, all emails, leads to this development style, which is
> > what
> > > > we
> > > > > have to live with.
> > > > >
> > > > > In this case, it seems we had to try the code several ways and see
> it
> > > > > before we can decide. In an office, we might have decided in pair
> > > > > programming in 5 minutes, this is not what we have. That or
> architect
> > > > would
> > > > > have created some coding edict that imposes coding style.
> > > > >
> > > > > So this circling is all OK by me ;-)
> > > > >
> > > > > Gary
> > > > >
> > > > >
> > > > > >
> > > > > > Emmanuel Bourg
> > > > > >
> > > > > >
> > > > > >
> > > > > >
> > ---------------------------------------------------------------------
> > > > > > To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> > > > > <javascript:;>
> > > > > > For additional commands, e-mail: dev-help@commons.apache.org
> > > > > <javascript:;>
> > > > > >
> > > > > >
> > > > >
> > > > >
> > > > > --
> > > > > E-Mail: garydgregory@gmail.com <javascript:;> |
> ggregory@apache.org
> > > > > <javascript:;>
> > > > > 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
> > >
> >
> >
> >
> > --
> > 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
>



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

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