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
