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
