[jira] [Commented] (CSV-68) Use Builder pattern for CSVFormat
[ https://issues.apache.org/jira/browse/CSV-68?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13497255#comment-13497255 ] Gary Gregory commented on CSV-68: - Hello Benedikt, I'm playing a bit of the devil's advocate here, so please bear with me. # Why are we still using CSVFormat.DISABLED? It feels left over from the pre-patch impl. # Why not get rid of pristine() and use defaults() instead? That automatically addresses (1). YW and thank you for the patch :) Gary > Use Builder pattern for CSVFormat > - > > Key: CSV-68 > URL: https://issues.apache.org/jira/browse/CSV-68 > Project: Commons CSV > Issue Type: Improvement >Reporter: Sebb > Attachments: CSV-68_2012.patch, CSV-68.patch, CSVFormat2.java, > CVSFormat2Main.java > > > Using a builder pattern to create CSVFormat instances would allow the > settings to be validated at creation time and would eliminate the need to > keep creating new CSVFormat instances whilst still allowing the class to be > immutable. > A possible API is as follows: > {code} > CSVFormat DEFAULT = CSVFormat.init(',') // delimiter is required > .withEncapsulator('"') > .withLeadingSpacesIgnored(true) > .withTrailingSpacesIgnored(true) > .withEmptyLinesIgnored(true) > .withLineSeparator("\r\n") // optional, as it would be the default > .build(); > CSVFormat format = CSVFormat.init(CSVFormat.DEFAULT) // alternatively start > with pre-defined format > .withSurroundingSpacesIgnored(false) > .build(); > {code} > Compare this with the current syntax: > {code} > // internal syntax; not easy to determine what all the parameters do > CSVFormat DEFAULT1 = new CSVFormat(',', '"', DISABLED, DISABLED, true, true, > false, true, CRLF); > // external syntax > CSVFormat format = CSVFormat.DEFAULT.withSurroundingSpacesIgnored(false); > {code} > As a proof of concept I've written skeleton code which compiles (but needs > completing). -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (CSV-68) Use Builder pattern for CSVFormat
[ https://issues.apache.org/jira/browse/CSV-68?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13497212#comment-13497212 ] Benedikt Ritter commented on CSV-68: Hi Gary, the problem is, that the user has no possibility to make sure that a CSVFormat is valid. Internally we use the package private validate() method for this purpose. Using a builder makes sure that only valid formats can be created and there is no need to validate a format after construction. Another advantage of the builder is, that CSVFormat's interface is smaller and does only contain methods that are needs when working with CSVFormats. OTH you can not simply call one of the "withXXX" methods to create a copy of a format. Regards and thanks for review, Benedikt > Use Builder pattern for CSVFormat > - > > Key: CSV-68 > URL: https://issues.apache.org/jira/browse/CSV-68 > Project: Commons CSV > Issue Type: Improvement >Reporter: Sebb > Attachments: CSV-68_2012.patch, CSV-68.patch, CSVFormat2.java, > CVSFormat2Main.java > > > Using a builder pattern to create CSVFormat instances would allow the > settings to be validated at creation time and would eliminate the need to > keep creating new CSVFormat instances whilst still allowing the class to be > immutable. > A possible API is as follows: > {code} > CSVFormat DEFAULT = CSVFormat.init(',') // delimiter is required > .withEncapsulator('"') > .withLeadingSpacesIgnored(true) > .withTrailingSpacesIgnored(true) > .withEmptyLinesIgnored(true) > .withLineSeparator("\r\n") // optional, as it would be the default > .build(); > CSVFormat format = CSVFormat.init(CSVFormat.DEFAULT) // alternatively start > with pre-defined format > .withSurroundingSpacesIgnored(false) > .build(); > {code} > Compare this with the current syntax: > {code} > // internal syntax; not easy to determine what all the parameters do > CSVFormat DEFAULT1 = new CSVFormat(',', '"', DISABLED, DISABLED, true, true, > false, true, CRLF); > // external syntax > CSVFormat format = CSVFormat.DEFAULT.withSurroundingSpacesIgnored(false); > {code} > As a proof of concept I've written skeleton code which compiles (but needs > completing). -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (CSV-68) Use Builder pattern for CSVFormat
[ https://issues.apache.org/jira/browse/CSV-68?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13497178#comment-13497178 ] Gary Gregory commented on CSV-68: - Hm... so why is this: {code:java} CSVFormat format = CSVFormat.defaults().withIgnoreSurroundingSpaces(false).build(); {code} better than: {code:java} CSVFormat format = CSVFormat.DEFAULT.withIgnoreSurroundingSpaces(false); {code} ? >From a user's POV, I see defaults() vs DEFAULT, I'm indifferent, but now I >have to call an extra method, build(). So the patch makes configuration MORE >verbose. That does not sound right. Does it to anyone else? G > Use Builder pattern for CSVFormat > - > > Key: CSV-68 > URL: https://issues.apache.org/jira/browse/CSV-68 > Project: Commons CSV > Issue Type: Improvement >Reporter: Sebb > Attachments: CSV-68_2012.patch, CSV-68.patch, CSVFormat2.java, > CVSFormat2Main.java > > > Using a builder pattern to create CSVFormat instances would allow the > settings to be validated at creation time and would eliminate the need to > keep creating new CSVFormat instances whilst still allowing the class to be > immutable. > A possible API is as follows: > {code} > CSVFormat DEFAULT = CSVFormat.init(',') // delimiter is required > .withEncapsulator('"') > .withLeadingSpacesIgnored(true) > .withTrailingSpacesIgnored(true) > .withEmptyLinesIgnored(true) > .withLineSeparator("\r\n") // optional, as it would be the default > .build(); > CSVFormat format = CSVFormat.init(CSVFormat.DEFAULT) // alternatively start > with pre-defined format > .withSurroundingSpacesIgnored(false) > .build(); > {code} > Compare this with the current syntax: > {code} > // internal syntax; not easy to determine what all the parameters do > CSVFormat DEFAULT1 = new CSVFormat(',', '"', DISABLED, DISABLED, true, true, > false, true, CRLF); > // external syntax > CSVFormat format = CSVFormat.DEFAULT.withSurroundingSpacesIgnored(false); > {code} > As a proof of concept I've written skeleton code which compiles (but needs > completing). -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (CSV-68) Use Builder pattern for CSVFormat
[ https://issues.apache.org/jira/browse/CSV-68?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13231930#comment-13231930 ] Sebb commented on CSV-68: - I've just discovered another disadvantage of the current code. Removal of unicode escaping required changes to every single constructor call in CSVFormat, of which there are a lot. It was also necessary to be very careful to remove the correct parameter in the predefined formats and test cases. Similarly when adding a new parameter, a lot of code has to be changed. With the proposed builder pattern, the changes are much simpler, because there is only one constructor call to change (and its parameters are named, so it's obvious where it should be put). > Use Builder pattern for CSVFormat > - > > Key: CSV-68 > URL: https://issues.apache.org/jira/browse/CSV-68 > Project: Commons CSV > Issue Type: Improvement >Reporter: Sebb > Attachments: CSV-68.patch, CSVFormat2.java, CVSFormat2Main.java > > > Using a builder pattern to create CSVFormat instances would allow the > settings to be validated at creation time and would eliminate the need to > keep creating new CSVFormat instances whilst still allowing the class to be > immutable. > A possible API is as follows: > {code} > CSVFormat DEFAULT = CSVFormat.init(',') // delimiter is required > .withEncapsulator('"') > .withLeadingSpacesIgnored(true) > .withTrailingSpacesIgnored(true) > .withEmptyLinesIgnored(true) > .withLineSeparator("\r\n") // optional, as it would be the default > .build(); > CSVFormat format = CSVFormat.init(CSVFormat.DEFAULT) // alternatively start > with pre-defined format > .withSurroundingSpacesIgnored(false) > .build(); > {code} > Compare this with the current syntax: > {code} > // internal syntax; not easy to determine what all the parameters do > CSVFormat DEFAULT1 = new CSVFormat(',', '"', DISABLED, DISABLED, true, true, > false, true, CRLF); > // external syntax > CSVFormat format = CSVFormat.DEFAULT.withSurroundingSpacesIgnored(false); > {code} > As a proof of concept I've written skeleton code which compiles (but needs > completing). -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (CSV-68) Use Builder pattern for CSVFormat
[ https://issues.apache.org/jira/browse/CSV-68?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13231899#comment-13231899 ] Emmanuel Bourg commented on CSV-68: --- I suggest to remove the validation completely. No other CSV API do that, and the protection it provides is useless. Then the concept of an "invalid" format disappears and there is no need to create a builder. > Use Builder pattern for CSVFormat > - > > Key: CSV-68 > URL: https://issues.apache.org/jira/browse/CSV-68 > Project: Commons CSV > Issue Type: Improvement >Reporter: Sebb > Attachments: CSV-68.patch, CSVFormat2.java, CVSFormat2Main.java > > > Using a builder pattern to create CSVFormat instances would allow the > settings to be validated at creation time and would eliminate the need to > keep creating new CSVFormat instances whilst still allowing the class to be > immutable. > A possible API is as follows: > {code} > CSVFormat DEFAULT = CSVFormat.init(',') // delimiter is required > .withEncapsulator('"') > .withLeadingSpacesIgnored(true) > .withTrailingSpacesIgnored(true) > .withEmptyLinesIgnored(true) > .withLineSeparator("\r\n") // optional, as it would be the default > .build(); > CSVFormat format = CSVFormat.init(CSVFormat.DEFAULT) // alternatively start > with pre-defined format > .withSurroundingSpacesIgnored(false) > .build(); > {code} > Compare this with the current syntax: > {code} > // internal syntax; not easy to determine what all the parameters do > CSVFormat DEFAULT1 = new CSVFormat(',', '"', DISABLED, DISABLED, true, true, > false, true, CRLF); > // external syntax > CSVFormat format = CSVFormat.DEFAULT.withSurroundingSpacesIgnored(false); > {code} > As a proof of concept I've written skeleton code which compiles (but needs > completing). -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (CSV-68) Use Builder pattern for CSVFormat
[ https://issues.apache.org/jira/browse/CSV-68?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13231794#comment-13231794 ] Sebb commented on CSV-68: - OK, let's use build, which is only two characters longer. Is it really that important to save 2 characters? == The current validate method cannot be called in the CSVFormat ctor, because the fluent chain may create an invalid instance part way through. Therefore the format method has to be called outside the class; it's very easy for the call to be omitted when the code is later maintained. It's not just so that the validate can be invoked earlier; it's to guarantee that an invalid format cannot be created. > Use Builder pattern for CSVFormat > - > > Key: CSV-68 > URL: https://issues.apache.org/jira/browse/CSV-68 > Project: Commons CSV > Issue Type: Improvement >Reporter: Sebb > Attachments: CSV-68.patch, CSVFormat2.java, CVSFormat2Main.java > > > Using a builder pattern to create CSVFormat instances would allow the > settings to be validated at creation time and would eliminate the need to > keep creating new CSVFormat instances whilst still allowing the class to be > immutable. > A possible API is as follows: > {code} > CSVFormat DEFAULT = CSVFormat.init(',') // delimiter is required > .withEncapsulator('"') > .withLeadingSpacesIgnored(true) > .withTrailingSpacesIgnored(true) > .withEmptyLinesIgnored(true) > .withLineSeparator("\r\n") // optional, as it would be the default > .build(); > CSVFormat format = CSVFormat.init(CSVFormat.DEFAULT) // alternatively start > with pre-defined format > .withSurroundingSpacesIgnored(false) > .build(); > {code} > Compare this with the current syntax: > {code} > // internal syntax; not easy to determine what all the parameters do > CSVFormat DEFAULT1 = new CSVFormat(',', '"', DISABLED, DISABLED, true, true, > false, true, CRLF); > // external syntax > CSVFormat format = CSVFormat.DEFAULT.withSurroundingSpacesIgnored(false); > {code} > As a proof of concept I've written skeleton code which compiles (but needs > completing). -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (CSV-68) Use Builder pattern for CSVFormat
[ https://issues.apache.org/jira/browse/CSV-68?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13231522#comment-13231522 ] Emmanuel Bourg commented on CSV-68: --- Ok it's shorter but it doesn't make sense, what is the meaning of a "go" action on a format? Also now every withXXX() method is duplicated in the code. I find this far fetched just to ensure that in the unlikely case of someone using the same character for the delimiter, encapsulator or escape, a warning will be thrown *ONE* line earlier. > Use Builder pattern for CSVFormat > - > > Key: CSV-68 > URL: https://issues.apache.org/jira/browse/CSV-68 > Project: Commons CSV > Issue Type: Improvement >Reporter: Sebb > Attachments: CSV-68.patch, CSVFormat2.java, CVSFormat2Main.java > > > Using a builder pattern to create CSVFormat instances would allow the > settings to be validated at creation time and would eliminate the need to > keep creating new CSVFormat instances whilst still allowing the class to be > immutable. > A possible API is as follows: > {code} > CSVFormat DEFAULT = CSVFormat.init(',') // delimiter is required > .withEncapsulator('"') > .withLeadingSpacesIgnored(true) > .withTrailingSpacesIgnored(true) > .withEmptyLinesIgnored(true) > .withLineSeparator("\r\n") // optional, as it would be the default > .build(); > CSVFormat format = CSVFormat.init(CSVFormat.DEFAULT) // alternatively start > with pre-defined format > .withSurroundingSpacesIgnored(false) > .build(); > {code} > Compare this with the current syntax: > {code} > // internal syntax; not easy to determine what all the parameters do > CSVFormat DEFAULT1 = new CSVFormat(',', '"', DISABLED, DISABLED, true, true, > false, true, CRLF); > // external syntax > CSVFormat format = CSVFormat.DEFAULT.withSurroundingSpacesIgnored(false); > {code} > As a proof of concept I've written skeleton code which compiles (but needs > completing). -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (CSV-68) Use Builder pattern for CSVFormat
[ https://issues.apache.org/jira/browse/CSV-68?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13231440#comment-13231440 ] Sebb commented on CSV-68: - How about: {code} CSVFormat.withDelimiter(';').withEncapsulator('"').go(); {code} It's one character shorter than your example. > Use Builder pattern for CSVFormat > - > > Key: CSV-68 > URL: https://issues.apache.org/jira/browse/CSV-68 > Project: Commons CSV > Issue Type: Improvement >Reporter: Sebb > Attachments: CSV-68.patch > > > Using a builder pattern to create CSVFormat instances would allow the > settings to be validated at creation time and would eliminate the need to > keep creating new CSVFormat instances whilst still allowing the class to be > immutable. > A possible API is as follows: > {code} > CSVFormat DEFAULT = CSVFormat.init(',') // delimiter is required > .withEncapsulator('"') > .withLeadingSpacesIgnored(true) > .withTrailingSpacesIgnored(true) > .withEmptyLinesIgnored(true) > .withLineSeparator("\r\n") // optional, as it would be the default > .build(); > CSVFormat format = CSVFormat.init(CSVFormat.DEFAULT) // alternatively start > with pre-defined format > .withSurroundingSpacesIgnored(false) > .build(); > {code} > Compare this with the current syntax: > {code} > // internal syntax; not easy to determine what all the parameters do > CSVFormat DEFAULT1 = new CSVFormat(',', '"', DISABLED, DISABLED, true, true, > false, true, CRLF); > // external syntax > CSVFormat format = CSVFormat.DEFAULT.withSurroundingSpacesIgnored(false); > {code} > As a proof of concept I've written skeleton code which compiles (but needs > completing). -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (CSV-68) Use Builder pattern for CSVFormat
[ https://issues.apache.org/jira/browse/CSV-68?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13231390#comment-13231390 ] Emmanuel Bourg commented on CSV-68: --- This one would satisfy me: {code} new CSVFormat().withDelimiter(';').withEncapsulator('"'); {code} > Use Builder pattern for CSVFormat > - > > Key: CSV-68 > URL: https://issues.apache.org/jira/browse/CSV-68 > Project: Commons CSV > Issue Type: Improvement >Reporter: Sebb > Attachments: CSV-68.patch > > > Using a builder pattern to create CSVFormat instances would allow the > settings to be validated at creation time and would eliminate the need to > keep creating new CSVFormat instances whilst still allowing the class to be > immutable. > A possible API is as follows: > {code} > CSVFormat DEFAULT = CSVFormat.init(',') // delimiter is required > .withEncapsulator('"') > .withLeadingSpacesIgnored(true) > .withTrailingSpacesIgnored(true) > .withEmptyLinesIgnored(true) > .withLineSeparator("\r\n") // optional, as it would be the default > .build(); > CSVFormat format = CSVFormat.init(CSVFormat.DEFAULT) // alternatively start > with pre-defined format > .withSurroundingSpacesIgnored(false) > .build(); > {code} > Compare this with the current syntax: > {code} > // internal syntax; not easy to determine what all the parameters do > CSVFormat DEFAULT1 = new CSVFormat(',', '"', DISABLED, DISABLED, true, true, > false, true, CRLF); > // external syntax > CSVFormat format = CSVFormat.DEFAULT.withSurroundingSpacesIgnored(false); > {code} > As a proof of concept I've written skeleton code which compiles (but needs > completing). -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (CSV-68) Use Builder pattern for CSVFormat
[ https://issues.apache.org/jira/browse/CSV-68?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13231369#comment-13231369 ] Sebb commented on CSV-68: - bq. how about renaming init(char) to withDelimiter(char) The delimiter is different, in that it is the only parameter that must be specified. But one could still potentially have a withDelimiter() method that could be applied to an existing Builder chain, e.g. to modify an existing format. [The builder delim field is final. I might well change that and add the method.] Note that the first and last methods in the chain have different input/output classes, so should use a different naming convention. "CSVFormat.init()" could be replaced by "new CSVFormat.Builder()" but I thought it was slightly neater to have a static "newInstance()" method, and given that the delimiter needs to be defined, I thought it might as well be a parameter to the init() method. The init() method can of course be renamed. It cannot be renamed to withDelimiter() unless the existing method of the same name is dropped; but even without the name clash I think it would be a mistake to use a name prefixed "with" as the "newInstance" method as users would expect to be able to provide the parameters in any order. Just realised that this would also be possible if each withMethod were defined as a static method in CSVFormat as well as a class method in the Builder. [Only the build() method would be unique to the Builder.] The only extra method call required would then be the build() method. So the syntax would be {code} CSVFormat.withDelimiter(',').withEncapsulator('"')...build(); CSVFormat.excel().withDelimiter(';').build(); {code} Maybe that would satisfy everyone? > Use Builder pattern for CSVFormat > - > > Key: CSV-68 > URL: https://issues.apache.org/jira/browse/CSV-68 > Project: Commons CSV > Issue Type: Improvement >Reporter: Sebb > Attachments: CSV-68.patch > > > Using a builder pattern to create CSVFormat instances would allow the > settings to be validated at creation time and would eliminate the need to > keep creating new CSVFormat instances whilst still allowing the class to be > immutable. > A possible API is as follows: > {code} > CSVFormat DEFAULT = CSVFormat.init(',') // delimiter is required > .withEncapsulator('"') > .withLeadingSpacesIgnored(true) > .withTrailingSpacesIgnored(true) > .withEmptyLinesIgnored(true) > .withLineSeparator("\r\n") // optional, as it would be the default > .build(); > CSVFormat format = CSVFormat.init(CSVFormat.DEFAULT) // alternatively start > with pre-defined format > .withSurroundingSpacesIgnored(false) > .build(); > {code} > Compare this with the current syntax: > {code} > // internal syntax; not easy to determine what all the parameters do > CSVFormat DEFAULT1 = new CSVFormat(',', '"', DISABLED, DISABLED, true, true, > false, true, CRLF); > // external syntax > CSVFormat format = CSVFormat.DEFAULT.withSurroundingSpacesIgnored(false); > {code} > As a proof of concept I've written skeleton code which compiles (but needs > completing). -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (CSV-68) Use Builder pattern for CSVFormat
[ https://issues.apache.org/jira/browse/CSV-68?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13231157#comment-13231157 ] Simone Tripodi commented on CSV-68: --- FWIW, +1 to builder and fluent APIs > Use Builder pattern for CSVFormat > - > > Key: CSV-68 > URL: https://issues.apache.org/jira/browse/CSV-68 > Project: Commons CSV > Issue Type: Improvement >Reporter: Sebb > Attachments: CSV-68.patch > > > Using a builder pattern to create CSVFormat instances would allow the > settings to be validated at creation time and would eliminate the need to > keep creating new CSVFormat instances whilst still allowing the class to be > immutable. > A possible API is as follows: > {code} > CSVFormat DEFAULT = CSVFormat.init(',') // delimiter is required > .withEncapsulator('"') > .withLeadingSpacesIgnored(true) > .withTrailingSpacesIgnored(true) > .withEmptyLinesIgnored(true) > .withLineSeparator("\r\n") // optional, as it would be the default > .build(); > CSVFormat format = CSVFormat.init(CSVFormat.DEFAULT) // alternatively start > with pre-defined format > .withSurroundingSpacesIgnored(false) > .build(); > {code} > Compare this with the current syntax: > {code} > // internal syntax; not easy to determine what all the parameters do > CSVFormat DEFAULT1 = new CSVFormat(',', '"', DISABLED, DISABLED, true, true, > false, true, CRLF); > // external syntax > CSVFormat format = CSVFormat.DEFAULT.withSurroundingSpacesIgnored(false); > {code} > As a proof of concept I've written skeleton code which compiles (but needs > completing). -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (CSV-68) Use Builder pattern for CSVFormat
[ https://issues.apache.org/jira/browse/CSV-68?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13231116#comment-13231116 ] Benedikt Ritter commented on CSV-68: Hey, how about renaming {{init(char)}} to {{withDelimiter(char)}} and then proving methods like: {code:java} public static Builder rfc4180() // (I never understood why it is called default...) public static Builder excel() public static Builder tdf() {code} Benedikt > Use Builder pattern for CSVFormat > - > > Key: CSV-68 > URL: https://issues.apache.org/jira/browse/CSV-68 > Project: Commons CSV > Issue Type: Improvement >Reporter: Sebb > Attachments: CSV-68.patch > > > Using a builder pattern to create CSVFormat instances would allow the > settings to be validated at creation time and would eliminate the need to > keep creating new CSVFormat instances whilst still allowing the class to be > immutable. > A possible API is as follows: > {code} > CSVFormat DEFAULT = CSVFormat.init(',') // delimiter is required > .withEncapsulator('"') > .withLeadingSpacesIgnored(true) > .withTrailingSpacesIgnored(true) > .withEmptyLinesIgnored(true) > .withLineSeparator("\r\n") // optional, as it would be the default > .build(); > CSVFormat format = CSVFormat.init(CSVFormat.DEFAULT) // alternatively start > with pre-defined format > .withSurroundingSpacesIgnored(false) > .build(); > {code} > Compare this with the current syntax: > {code} > // internal syntax; not easy to determine what all the parameters do > CSVFormat DEFAULT1 = new CSVFormat(',', '"', DISABLED, DISABLED, true, true, > false, true, CRLF); > // external syntax > CSVFormat format = CSVFormat.DEFAULT.withSurroundingSpacesIgnored(false); > {code} > As a proof of concept I've written skeleton code which compiles (but needs > completing). -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (CSV-68) Use Builder pattern for CSVFormat
[ https://issues.apache.org/jira/browse/CSV-68?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13231073#comment-13231073 ] Sebb commented on CSV-68: - The extra verbosity is .init(',') and .build() Not a lot. The gains are: - much clearer setup of built-in formats - guaranteed validation > Use Builder pattern for CSVFormat > - > > Key: CSV-68 > URL: https://issues.apache.org/jira/browse/CSV-68 > Project: Commons CSV > Issue Type: Improvement >Reporter: Sebb > Attachments: CSV-68.patch > > > Using a builder pattern to create CSVFormat instances would allow the > settings to be validated at creation time and would eliminate the need to > keep creating new CSVFormat instances whilst still allowing the class to be > immutable. > A possible API is as follows: > {code} > CSVFormat DEFAULT = CSVFormat.init(',') // delimiter is required > .withEncapsulator('"') > .withLeadingSpacesIgnored(true) > .withTrailingSpacesIgnored(true) > .withEmptyLinesIgnored(true) > .withLineSeparator("\r\n") // optional, as it would be the default > .build(); > CSVFormat format = CSVFormat.init(CSVFormat.DEFAULT) // alternatively start > with pre-defined format > .withSurroundingSpacesIgnored(false) > .build(); > {code} > Compare this with the current syntax: > {code} > // internal syntax; not easy to determine what all the parameters do > CSVFormat DEFAULT1 = new CSVFormat(',', '"', DISABLED, DISABLED, true, true, > false, true, CRLF); > // external syntax > CSVFormat format = CSVFormat.DEFAULT.withSurroundingSpacesIgnored(false); > {code} > As a proof of concept I've written skeleton code which compiles (but needs > completing). -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (CSV-68) Use Builder pattern for CSVFormat
[ https://issues.apache.org/jira/browse/CSV-68?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13230948#comment-13230948 ] Emmanuel Bourg commented on CSV-68: --- As stated on the list I'm -1 on changing CSVFormat to this form, it's more verbose for no substantial gain. The delayed validation is not an issue. > Use Builder pattern for CSVFormat > - > > Key: CSV-68 > URL: https://issues.apache.org/jira/browse/CSV-68 > Project: Commons CSV > Issue Type: Improvement >Reporter: Sebb > Attachments: CSV-68.patch > > > Using a builder pattern to create CSVFormat instances would allow the > settings to be validated at creation time and would eliminate the need to > keep creating new CSVFormat instances whilst still allowing the class to be > immutable. > A possible API is as follows: > {code} > CSVFormat DEFAULT = CSVFormat.init(',') // delimiter is required > .withEncapsulator('"') > .withLeadingSpacesIgnored(true) > .withTrailingSpacesIgnored(true) > .withEmptyLinesIgnored(true) > .withLineSeparator("\r\n") // optional, as it would be the default > .build(); > CSVFormat format = CSVFormat.init(CSVFormat.DEFAULT) // alternatively start > with pre-defined format > .withSurroundingSpacesIgnored(false) > .build(); > {code} > Compare this with the current syntax: > {code} > // internal syntax; not easy to determine what all the parameters do > CSVFormat DEFAULT1 = new CSVFormat(',', '"', DISABLED, DISABLED, true, true, > false, true, CRLF); > // external syntax > CSVFormat format = CSVFormat.DEFAULT.withSurroundingSpacesIgnored(false); > {code} > As a proof of concept I've written skeleton code which compiles (but needs > completing). -- This message is automatically generated by JIRA. If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa For more information on JIRA, see: http://www.atlassian.com/software/jira