On Aug 18, 2017 16:10, "Gilles" <gil...@harfang.homelinux.org> wrote:
On Fri, 18 Aug 2017 15:46:11 -0600, Gary Gregory wrote: > On Fri, Aug 18, 2017 at 3:38 PM, Gilles <gil...@harfang.homelinux.org> > wrote: > > On Fri, 18 Aug 2017 13:21:45 -0600, Gary Gregory wrote: >> >> On Fri, Aug 18, 2017 at 12:50 PM, Gilles <gil...@harfang.homelinux.org> >>> wrote: >>> >>> On Fri, 18 Aug 2017 12:41:01 -0600, Gary Gregory wrote: >>> >>>> >>>> On Wed, Aug 16, 2017 at 6:28 PM, Gilles <gil...@harfang.homelinux.org> >>>> >>>>> wrote: >>>>> >>>>> On Wed, 16 Aug 2017 11:27:53 -0600, Gary Gregory wrote: >>>>> >>>>> >>>>>> Let's summarize the options: >>>>>> >>>>>> >>>>>>> 0) do nothing >>>>>>> 1) Add two put methods to CVSRecord making the class mutable >>>>>>> 2) Add a "mutableRecord" boolean option to CVSRecord and CSVFormat >>>>>>> such >>>>>>> that a new boolean in CVSRecord allow method from 1) above to either >>>>>>> work >>>>>>> or throw an exception. >>>>>>> 3) Add a "mutableRecord" boolean option to CVSRecord and CSVFormat >>>>>>> such >>>>>>> that subclass of CVSRecord called CVSMutableRecord is created which >>>>>>> contains two new put methods >>>>>>> >>>>>>> What else? >>>>>>> >>>>>>> >>>>>>> 4) The factory method: >>>>>>> >>>>>> /** >>>>>> * @param orig Original to be copied. >>>>>> * @param replace Fields to be replaced. >>>>>> * @return a copy of "orig", except for the fields in "replace". >>>>>> */ >>>>>> public static CSVRecord createRecord(CSVRecord orig, >>>>>> Pair<Integer, String> ... >>>>>> replace) >>>>>> >>>>>> Could also be: >>>>>> public static CSVRecord createRecord(CSVRecord orig, >>>>>> int[] replaceIndices, >>>>>> String[] replaceValues) >>>>>> >>>>>> >>>>>> >>>>> To me, that feels like bending over backwards and hard to very ugly to >>>>> use, >>>>> building an array of ints, building an array of Strings. Yikes. But, >>>>> hey >>>>> that's just me. >>>>> >>>>> >>>>> What about the "Pair" alternative above? >>>> >>>> Another alternative would be to have a "CSVRecordBuilder" (with "put" >>>> methods): >>>> ---CUT--- >>>> CSVRecord rec = readRecord(source); >>>> rec = new CSVRecordBuilder(rec).put(1, "Change").put(4, "this").build(); >>>> ---CUT--- >>>> >>>> >>>> For my money, the above is convoluted for no reason from a user's POV, >>> >>> >> I surely agree that the "builder" approach is less obvious >> than adding methods as the need seems to arise. >> But the part "for no reason" is simply not true. >> >> My opinion is that a library is more useful if it limits >> the number of ways one can shoot one's foot. YMMV. :-) >> >> What are the consequences in various use-cases? >> Is mutability or immutability useful in selected cases? >> And from there, how to modify the design to gracefully >> handle all of them? >> >> Assuming that you want to define a mutable class for >> some of those cases, it might make sense to provide >> a factory to create an immutable instance: >> >> public class CSVRecord { >> >> /** Deep copy constructor. */ >> public CSVRecord(CSVRecord rec) { /* ... */ } >> >> public void put(int i, String s) { /* ... */ } >> >> /** Create an immutable copy. */ >> public CSVRecord createImmutable() { >> return new CSVRecord(this) { >> public void put(int i, String s) { >> throw new UnsupportedOperationException(); >> } >> } >> } >> } >> >> That way, in the new release, users that relied on the >> immutable character of "CSVRecord" can at least modify >> their code at the (hopefully few) right places and still >> maintain consistency. >> > > > I did something like that in the branch CSV-216 but that tweaks a lot of > code and not every one liked that. > Perhaps because "mutable or not" is not part of the format... Here the immutability is provided on a per-record basis. Anyways, I'm not even sure that it's a good enough substitute for compiler-enforced immutability. :-{ Finally, it boils down to what exactly the class "CSVRecord" represents. Your view is definitely different from the original designers' (perhaps a younger you was among them?). You are asking a lot of me as I can't recall what I had for breakfast ;-) When I started contributing to CVS, my use cases where read-only, they still are, but I am happy to accommodate RW use cases, one way or another. I just do not see doing it on a per record basis. Gary Gilles Gary > > >> >> Gilles >> >> again compared to the simple: >> >>> >>> rec.put(1, "Change"); >>> rec.put(4, "this"); >>> >>> Gary >>> >>> >>> >>>> >>>> >> --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org For additional commands, e-mail: dev-h...@commons.apache.org