[
https://issues.apache.org/jira/browse/SANDBOX-279?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Bob Smith updated SANDBOX-279:
------------------------------
Attachment: CSVStrategy_Immutable.patch
If its still ok to make a big API change, then I think that would be a great
idea. And if CSVStrategy is immutable then there would be no need to ever copy
a CSVStrategy (so the clone method could go away and a copy constructor would
not have to be added).
If that change is made, then it might be nice to use the Builder pattern and
get rid of all the normal constructors. That would keep it from loosing the
flexibility that the mutable version had and it also makes it possible to add
new values to CSVStrategy in the future without having to add more
constructors. And I think it would make it easier to tell what parameter each
each value goes with (especially with so many boolean variables).
I attached a patch that includes these changes. Surprisingly, there weren't
that many changes needed to the rest of the classes. A deprecated constructor
in CSVParser needed to be changed to use a CSVStrategy.Builder and a few of the
CSVParser test cases needed similar changes. I also got rid of the CSVParser
test cases for the getters/setters of the CSVStrategy methods, but I suppose
they could be modified to test the Builder inetead.
> CSVStrategy has modifiable public static variables
> --------------------------------------------------
>
> Key: SANDBOX-279
> URL: https://issues.apache.org/jira/browse/SANDBOX-279
> Project: Commons Sandbox
> Issue Type: Improvement
> Components: CSV
> Affects Versions: Nightly Builds
> Reporter: Bob Smith
> Priority: Minor
> Attachments: CSVStrategy_Immutable.patch, CSVStrategyAndTests.patch
>
>
> The public static variables in CSVStrategy are not final and
> DEFAULT_STRATEGY, EXCEL_STRATEGY, and TDF_STRATEGY can be modified using
> setter methods. I would recommend making them all final and using an
> unmodifiable subclass for the CSVStrategy variables.
> For this change to work, the main CSVStrategy constructor would have to be
> changed to set the class fields directly instead of using the setters since
> the unmodifiable subclass will have overwritten all of the setters to thrown
> UnsupportedOperationExceptions. And I think a copy constructor would also be
> a good addition to allow easily copying one of these (or any) CSVStrategy
> objects if any modifications have to be made to them (as an alternative to
> the clone method).
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
