From my perspective, if DEFAULT_STRATEGY, EXCEL_STRATEGY, or TDF_STRATEGY are
mutable, then copies of them should be returned in a factory method. Ideally, a
selector would be passed to a factory method:
CSVStrategy strategy = CSVStrategy.getInstance(CSVStrategy.DEFAULT_STRATEGY);
-Adrian
On 1/29/2011 8:51 PM, Jacopo Cappellato 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
