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" <simonetrip...@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.cappell...@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.cappell...@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-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
>>>> 
>>> 
>>> 
>>> ---------------------------------------------------------------------
>>> 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
>> 
> 
> ---------------------------------------------------------------------
> 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

Reply via email to