On Mon, Jul 21, 2014 at 12:35 PM, Benedikt Ritter <brit...@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 <garydgreg...@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 < > > brit...@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 <dipanja...@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 <garydgreg...@gmail.com> > wrote: > > > > > > > On Sat, Jul 19, 2014 at 12:14 PM, Emmanuel Bourg <ebo...@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-unsubscr...@commons.apache.org > > > > <javascript:;> > > > > > For additional commands, e-mail: dev-h...@commons.apache.org > > > > <javascript:;> > > > > > > > > > > > > > > > > > > > > > > -- > > > > E-Mail: garydgreg...@gmail.com <javascript:;> | ggreg...@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: 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
