Re: [CSV] Discussion about the new CSVFormatBuilder
Maybe we first have to decide if we want validation of CSVFormats at construction time or not. If not, the changes of CSV-68 can be reverted. Benedikt 2012/11/21 James Carman ja...@carmanconsulting.com I don't really have a problem with the extra call to build() before you have something useful. It does give us the ability to do validation on the object before you build it. If we choose not to do the validation at this time, that's fine, but if we ever do choose to add that in the future, we don't have to break API backward compatibility to do so. On Tue, Nov 20, 2012 at 5:57 PM, Gary Gregory garydgreg...@gmail.com wrote: Ok this is good. Let's see some healthy debating. :) What is the alternate API? To me the bother is the extra build() call, but that's the pattern. Could an alt API be used and co-exist? Is making the ctor an option? It would have to do some validation. Gary On Nov 20, 2012, at 16:59, Emmanuel Bourg ebo...@apache.org wrote: Le 20/11/2012 20:01, Benedikt Ritter a écrit : Please share your thoughts about the builder. Sorry Benedikt but I have to say I really don't like this design. I prefer a simpler API for the reasons you mentioned in the disadvantages. The minor improvements from the developer's point of view are much less important than the ease of use from user's point of view. Emmanuel Bourg - To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org For additional commands, e-mail: dev-h...@commons.apache.org - To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org For additional commands, e-mail: dev-h...@commons.apache.org
Re: [CSV] Discussion about the new CSVFormatBuilder
I don't really have a problem with the extra call to build() before you have something useful. It does give us the ability to do validation on the object before you build it. If we choose not to do the validation at this time, that's fine, but if we ever do choose to add that in the future, we don't have to break API backward compatibility to do so. On Tue, Nov 20, 2012 at 5:57 PM, Gary Gregory garydgreg...@gmail.com wrote: Ok this is good. Let's see some healthy debating. :) What is the alternate API? To me the bother is the extra build() call, but that's the pattern. Could an alt API be used and co-exist? Is making the ctor an option? It would have to do some validation. Gary On Nov 20, 2012, at 16:59, Emmanuel Bourg ebo...@apache.org wrote: Le 20/11/2012 20:01, Benedikt Ritter a écrit : Please share your thoughts about the builder. Sorry Benedikt but I have to say I really don't like this design. I prefer a simpler API for the reasons you mentioned in the disadvantages. The minor improvements from the developer's point of view are much less important than the ease of use from user's point of view. Emmanuel Bourg - To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org For additional commands, e-mail: dev-h...@commons.apache.org - To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org For additional commands, e-mail: dev-h...@commons.apache.org
[CSV] Discussion about the new CSVFormatBuilder
Hi, Gary and I did some work on CSV-68 Use the Builder Pattern to create CSVFormats [1]. We have implemented a builder for CSVFormats in trunk. It is capable of... ...creating a CSVFormat from scratch by only passing in a delimiter: CSVFormat format = CSVFormat.newBuilder(',').build(); ...creating a CSVFormat from the commons-csv defaults (RFC4180 but ignoring empty lines) CSVFormat format = CSVFormat.defaults().build(); ...creating a CSVFormat based on a CSVFormat copyOfRFC4180 = CSVFormat.newBuilder(CSVFormat.RFC4180).build(); The builders provides the same fluent API like CSVFormat did before by returning itself: CSVFormat format = CSVFormat.newBuilder(',').withRecordSeparator('\n').withIgnoreEmptyLines(true).build(); Using a builder has several advantages: - the construction of CSVFormats can be separated from the CSVFormat itself - the interface of CSVFormat is now much smaller - if we want to add another parameter to CSVFormat we only have to change CSVFormat's private ctor, and the build method. We had to change every withXXX method before. - CSVFormats can only be created into a valid state, because the builder internally uses the validate method, which was package visible before and therefore not accessible to users. However there are also disadvantages: - the builder is more verbose; it forces users to the build() method, where every withXXX method returned a CSVFormat before. - it has been argued that using the builder pattern only to make sure CSVFormats are valid is overengineered. No other library has this kind of validation. Please share your thoughts about the builder. Regards, Benedikt [1] https://issues.apache.org/jira/browse/CSV-68
Re: [CSV] Discussion about the new CSVFormatBuilder
On Tue, Nov 20, 2012 at 2:01 PM, Benedikt Ritter benerit...@gmail.comwrote: Hi, Gary and I did some work on CSV-68 Use the Builder Pattern to create CSVFormats [1]. We have implemented a builder for CSVFormats in trunk. It is capable of... ...creating a CSVFormat from scratch by only passing in a delimiter: CSVFormat format = CSVFormat.newBuilder(',').build(); ...creating a CSVFormat from the commons-csv defaults (RFC4180 but ignoring empty lines) CSVFormat format = CSVFormat.defaults().build(); ...creating a CSVFormat based on a CSVFormat copyOfRFC4180 = CSVFormat.newBuilder(CSVFormat.RFC4180).build(); The builders provides the same fluent API like CSVFormat did before by returning itself: CSVFormat format = CSVFormat.newBuilder(',').withRecordSeparator('\n').withIgnoreEmptyLines(true).build(); Using a builder has several advantages: - the construction of CSVFormats can be separated from the CSVFormat itself - the interface of CSVFormat is now much smaller - if we want to add another parameter to CSVFormat we only have to change CSVFormat's private ctor, and the build method. We had to change every withXXX method before. - CSVFormats can only be created into a valid state, because the builder internally uses the validate method, which was package visible before and therefore not accessible to users. However there are also disadvantages: - the builder is more verbose; it forces users to the build() method, where every withXXX method returned a CSVFormat before. I'm not crazy about the extra method call but I see the value of the pattern, especially always having a valid state (which we also had before right?) The build() method could also be called toCSVFormat(). Like StringBuilder has toString(), a FooBuilder can say toFoo() so you know what to expect. Gary - it has been argued that using the builder pattern only to make sure CSVFormats are valid is overengineered. No other library has this kind of validation. Please share your thoughts about the builder. Regards, Benedikt [1] https://issues.apache.org/jira/browse/CSV-68 -- E-Mail: garydgreg...@gmail.com | ggreg...@apache.org JUnit in Action, 2nd Ed: http://goog_1249600977http://bit.ly/ECvg0 Spring Batch in Action: http://s.apache.org/HOqhttp://bit.ly/bqpbCK Blog: http://garygregory.wordpress.com Home: http://garygregory.com/ Tweet! http://twitter.com/GaryGregory
Re: [CSV] Discussion about the new CSVFormatBuilder
Surely you meant to say no other commons library. Builder patterns are relatively common. See guava for instance: http://docs.guava-libraries.googlecode.com/git/javadoc/com/google/common/base/Splitter.html On Tue, Nov 20, 2012 at 11:49 AM, Gary Gregory garydgreg...@gmail.comwrote: - it has been argued that using the builder pattern only to make sure CSVFormats are valid is overengineered. No other library has this kind of validation.
Re: [CSV] Discussion about the new CSVFormatBuilder
Hey Ted, no I was referring to the comments of CSV-68, where it was stated that no other CSV library provides validation of the used CSV formats (and hence it can be removed from commons csv entirely). Benedikt 2012/11/20 Ted Dunning ted.dunn...@gmail.com Surely you meant to say no other commons library. Builder patterns are relatively common. See guava for instance: http://docs.guava-libraries.googlecode.com/git/javadoc/com/google/common/base/Splitter.html On Tue, Nov 20, 2012 at 11:49 AM, Gary Gregory garydgreg...@gmail.com wrote: - it has been argued that using the builder pattern only to make sure CSVFormats are valid is overengineered. No other library has this kind of validation.
Re: [CSV] Discussion about the new CSVFormatBuilder
Le 20/11/2012 20:01, Benedikt Ritter a écrit : Please share your thoughts about the builder. Sorry Benedikt but I have to say I really don't like this design. I prefer a simpler API for the reasons you mentioned in the disadvantages. The minor improvements from the developer's point of view are much less important than the ease of use from user's point of view. Emmanuel Bourg smime.p7s Description: Signature cryptographique S/MIME
Re: [CSV] Discussion about the new CSVFormatBuilder
Ok this is good. Let's see some healthy debating. :) What is the alternate API? To me the bother is the extra build() call, but that's the pattern. Could an alt API be used and co-exist? Is making the ctor an option? It would have to do some validation. Gary On Nov 20, 2012, at 16:59, Emmanuel Bourg ebo...@apache.org wrote: Le 20/11/2012 20:01, Benedikt Ritter a écrit : Please share your thoughts about the builder. Sorry Benedikt but I have to say I really don't like this design. I prefer a simpler API for the reasons you mentioned in the disadvantages. The minor improvements from the developer's point of view are much less important than the ease of use from user's point of view. Emmanuel Bourg - To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org For additional commands, e-mail: dev-h...@commons.apache.org
Re: [CSV] Discussion about the new CSVFormatBuilder
Another way of looking at the builder style is that it is Java's way of using keyword arguments for complex constructors. It also allows a reasonable amount of future-proofing. These benefits are hard to replicate with constructors. On the other hand, builder-style patterns are a royal pain with serialization frameworks. On Tue, Nov 20, 2012 at 2:57 PM, Gary Gregory garydgreg...@gmail.comwrote: Ok this is good. Let's see some healthy debating. :) What is the alternate API? To me the bother is the extra build() call, but that's the pattern. Could an alt API be used and co-exist? Is making the ctor an option? It would have to do some validation. Gary On Nov 20, 2012, at 16:59, Emmanuel Bourg ebo...@apache.org wrote: Le 20/11/2012 20:01, Benedikt Ritter a écrit : Please share your thoughts about the builder. Sorry Benedikt but I have to say I really don't like this design. I prefer a simpler API for the reasons you mentioned in the disadvantages. The minor improvements from the developer's point of view are much less important than the ease of use from user's point of view. Emmanuel Bourg - To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org For additional commands, e-mail: dev-h...@commons.apache.org
Re: [CSV] Discussion about the new CSVFormatBuilder
Le 20/11/2012 23:57, Gary Gregory a écrit : Ok this is good. Let's see some healthy debating. :) Until the debate degrades my mental health... What is the alternate API? The pre CSV-68 API. To me the bother is the extra build() call, but that's the pattern. A pattern is not a feature or a goal, it's a way to solve a problem. There was no problem with the API. Could an alt API be used and co-exist? No thanks, ultimately I'm arguing for simplicity for the end users. If I wished more than one way to do the job I would do it in Perl (or Scala). Is making the ctor an option? It would have to do some validation. What about making CSVFormat.validate() public for validating after building the format if that's really required ? Emmanuel Bourg smime.p7s Description: Signature cryptographique S/MIME