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" <[email protected]> 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
>> <[email protected]> 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 <[email protected]>
>>>> 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: [email protected]
>>>>> For additional commands, e-mail: [email protected]
>>>>>
>>>>>
>>>>
>>>> ---------------------------------------------------------------------
>>>> To unsubscribe, e-mail: [email protected]
>>>> For additional commands, e-mail: [email protected]
>>>>
>>>
>>>
>>> ---------------------------------------------------------------------
>>> To unsubscribe, e-mail: [email protected]
>>> For additional commands, e-mail: [email protected]
>>>
>>>
>>
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: [email protected]
>> For additional commands, e-mail: [email protected]
>>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: [email protected]
> For additional commands, e-mail: [email protected]
>
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
