On 30 January 2011 09:09, Stephen Colebourne <scolebou...@joda.org> wrote:
> The modern standard for altering immutable classes was laid down by Joda-Time
>
> strategy = strategy.withCommentStart("#");
Not sure I undestand that.
> Really, though the question is whether it shoudl be immutable or not.
+1
Whatever decision is taken needs to be documented:
if the class is intended to be immutable, then this needs to be
documented so future maintainers don't break it, and so users know it
is thread-safe.
If not immutable, then the class thread-safety still needs to be documented.
> Stephen
>
>
> On 30 January 2011 04:51, Jacopo Cappellato <jacopo.cappell...@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" <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
>>
>>
>
> ---------------------------------------------------------------------
> 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
