commons-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Stephen Colebourne <scolebou...@joda.org>
Subject Re: [csv] Proposal to remove setter methods from CSVStrategy
Date Sun, 30 Jan 2011 09:09:42 GMT
The modern standard for altering immutable classes was laid down by Joda-Time

strategy = strategy.withCommentStart("#");

Really, though the question is whether it shoudl be immutable or not.

Stephen


On 30 January 2011 04:51, Jacopo Cappellato <jacopo.cappellato@gmail.com> wrote:
> Hi Gary,
>
> I initially proposed the change when I noticed the following code in one of the automated
tests of common-csv:
>
>    CSVStrategy strategy = (CSVStrategy)CSVStrategy.DEFAULT_STRATEGY.clone();
>    strategy.setCommentStart('#');
>    TestCSVParser parser = new TestCSVParser(new StringReader(code), strategy);
>
> and I realized the importance, in a calling method that alters the default settings,
of using "clone".
> But apart from this I don't have real world use cases that describe issues around this.
>
> Jacopo
>
>
> On Jan 29, 2011, at 9:49 PM, Gary Gregory wrote:
>
>> Are people really using these classes in an MT way?  These seem like the kind of
classes you do not share between threads. My op only though.
>>
>> Gary
>>
>> On Jan 29, 2011, at 12:27, "Simone Tripodi" <simonetripodi@apache.org> wrote:
>>
>>> Hi all guys!!! :)
>>> I'd suggest to apply Jacopo's suggestion, making strategies' fields as
>>> final would help classes becoming thread-safe; to fix the issue of
>>> construction described by Stephen, it would be useful adding builder
>>> classes (optionally implemented as static inner classes).
>>> WDYT? HTH, that's just my opinion,
>>> Simo
>>>
>>> http://people.apache.org/~simonetripodi/
>>> http://www.99soft.org/
>>>
>>>
>>>
>>> On Sat, Jan 29, 2011 at 2:33 PM, Jacopo Cappellato
>>> <jacopo.cappellato@gmail.com> wrote:
>>>> Thank you Stephen,
>>>>
>>>> I understand this; alternatively we could implement a factory method that
clones DEFAULT_STRATEGY, EXCEL_STRATEGY, TDF_STRATEGY; but I will definitely postpone any
decision, it is not a big deal for me and we can wait for additional feedback.
>>>>
>>>> Jacopo
>>>>
>>>> On Jan 29, 2011, at 1:16 PM, Stephen Colebourne wrote:
>>>>
>>>>> Traditionally, many commons projects has taken no position on the
>>>>> correct way to instantiate objects, whether via constructor or bean
>>>>> methods. This was to allow construction from tools such as Velocity.
>>>>> It is also still easier to construct using Spring via bean methods
>>>>> rather than the constructor.
>>>>>
>>>>> These days, immutability and concurrency are more important, so this
>>>>> change may be applicable, buts its good to understand the history and
>>>>> wide use cases of classes like these.
>>>>>
>>>>> Stephen
>>>>>
>>>>>
>>>>> On 29 January 2011 12:04, Jacopo Cappellato <jacopo.cappellato@gmail.com>
wrote:
>>>>>> Hi all,
>>>>>>
>>>>>> in my opinion all the setter methods should be removed from the CSVStrategy
class: in this way the fields will only be set using the constructors and they will become
readonly.
>>>>>> The main issue I see with the current implementation is that a calling
method can modify the values of the fields of the following static objects declared in CSVStrategy
(changing the default behavior for all subsequent code that uses for example CSVStrategy.DEFAULT_STRATEGY):
>>>>>>
>>>>>>   public static CSVStrategy DEFAULT_STRATEGY = new CSVStrategy(',',
'"', COMMENTS_DISABLED, ESCAPE_DISABLED, true,
>>>>>>                                            
                   true, false, true);
>>>>>>   public static CSVStrategy EXCEL_STRATEGY   = new CSVStrategy(',',
'"', COMMENTS_DISABLED, ESCAPE_DISABLED, false,
>>>>>>                                            
                   false, false, false);
>>>>>>   public static CSVStrategy TDF_STRATEGY     = new CSVStrategy('\t',
'"', COMMENTS_DISABLED, ESCAPE_DISABLED, true,
>>>>>>                                            
                   true, false, true);
>>>>>>
>>>>>> What do you think?
>>>>>>
>>>>>> Jacopo
>>>>>>
>>>>>>
>>>>>> ---------------------------------------------------------------------
>>>>>> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
>>>>>> For additional commands, e-mail: dev-help@commons.apache.org
>>>>>>
>>>>>>
>>>>>
>>>>> ---------------------------------------------------------------------
>>>>> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
>>>>> For additional commands, e-mail: dev-help@commons.apache.org
>>>>>
>>>>
>>>>
>>>> ---------------------------------------------------------------------
>>>> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
>>>> For additional commands, e-mail: dev-help@commons.apache.org
>>>>
>>>>
>>>
>>> ---------------------------------------------------------------------
>>> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
>>> For additional commands, e-mail: dev-help@commons.apache.org
>>>
>>
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
>> For additional commands, e-mail: dev-help@commons.apache.org
>>
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> For additional commands, e-mail: dev-help@commons.apache.org
>
>

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org


Mime
View raw message