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 19:05:07 GMT
2014-07-21 20:55 GMT+02:00 Gary Gregory <garydgregory@gmail.com>:

> On Mon, Jul 21, 2014 at 2:45 PM, Benedikt Ritter <britter@apache.org>
> wrote:
>
> > 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 ?
> >
>
> No good reason.
>

:)

How about removing the postfixes again?

If the requirement for additional getters returning different typs come up,
we can still add getters with postfix:

CSV v1.0:
  getCommentStart()

CSV v1.1
  getCommentStart()
  getCommentStartString() / getCommentStartAsString()

WDYT?

Benedikt


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