On Fri, Aug 18, 2017 at 12:41 PM, Gilles <gil...@harfang.homelinux.org>
wrote:
> On Fri, 18 Aug 2017 19:52:01 +0200, Gilles wrote:
>
>> On Fri, 18 Aug 2017 11:34:54 -0600, Gary Gregory wrote:
>>
>>> To be clear, you are then OK with simply adding the two put() methods to
>>> CSVRecord? Option 1 in my eariler message.
>>>
>>
>> Actually, I think that it is not OK to not acknowledge the
>> breaking nature of the proposal.
>> As Simon noted you might want to consider that such a change
>> warrants a major release, or not if you _decide_ that the
>> project never promised immutability (despite what the source
>> says through self-documentation).
>>
>> I'm not opposing a decision to break, if that's your preferred
>> option (despite the technical arguments against what I proposed
>> have not been substantiated IIUC).
>>
>
> By the above paragraph I mean that I don't remember reading why
> the following (pseudo-code) was not a good option:
> ---CUT---
> CSRecord rec = readRecord(source);
> rec = rec.copyAndReplace(new int[] {1, 4},
> new String[] {"Change", "this"});
> ---CUT---
>
The above is non-intuitive, this is clear and simple
rec.put(1, "Change");
rec.put(4, "this");
Gary
> Gilles
>
>
>
>> In any cases, a mutable subclass is not a solution IMO.
>>
>> Not sure I'm clear enough... :-)
>> Gilles
>>
>> Gary
>>>
>>> On Fri, Aug 18, 2017 at 10:05 AM, Gilles <gil...@harfang.homelinux.org>
>>> wrote:
>>>
>>> On Fri, 18 Aug 2017 09:36:06 -0600, Gary Gregory wrote:
>>>>
>>>> Please see branch CSV-216 for a KISS implementation that uses a
>>>>> CSVMutableRecord subclass.
>>>>>
>>>>>
>>>> I don't think anyone gains anything through this subclassing.
>>>>
>>>> A client can no longer assume that an instance of "CSVRecord" is
>>>> immutable and will have to make a defensive copy or an "instanceof"
>>>> check (that will be obsolete/broken whenever the hierarchy is
>>>> modified).
>>>>
>>>> Better assume a functionally breaking change and add the methods
>>>> to class "CSVRecord"...
>>>>
>>>> Gilles
>>>>
>>>>
>>>>
>>>> I do not believe this feature warrants creating interfaces or
>>>>> framework-like code. I do not believe we need to start leaning the
>>>>> JDBC-way.
>>>>>
>>>>> Gary
>>>>>
>>>>> On Thu, Aug 17, 2017 at 3:04 PM, Simon Spero <sesunc...@gmail.com>
>>>>> wrote:
>>>>>
>>>>> On Aug 15, 2017 8:01 PM, "Gilles" <gil...@harfang.homelinux.org>
>>>>> wrote:
>>>>>
>>>>>>
>>>>>> Saying that making record mutable is "breaking" is a bit unfair when
>>>>>> we
>>>>>> do
>>>>>> > NOT document the mutability of the class in the first place.
>>>>>> >
>>>>>>
>>>>>> I'm stating a fact: class is currently immutable, change would make it
>>>>>> mutable; it is functionally breaking.
>>>>>> I didn't say that you are forbidden to do it; just that it would be
>>>>>> unwise,
>>>>>> particularly if it would be to save a few bytes.
>>>>>>
>>>>>>
>>>>>> Exactly.
>>>>>>
>>>>>> TL;DR. This is almost always a breaking semantic change; the safest
>>>>>> ways
>>>>>> of implementing it are binary breaking; it's unlikely to have a major
>>>>>> performance impact; it might be better to create a new API module for
>>>>>> enhancements, with current package as legacy or implementation.
>>>>>>
>>>>>> If a class previously exposed no mutators, adding one is usually a
>>>>>> major
>>>>>> change. This is especially true for final classes, but it still
>>>>>> affects
>>>>>> use
>>>>>> cases where an instance is owned by another class, which may rely on
>>>>>> the
>>>>>> lack of mutability to avoid making defensive copies.
>>>>>> Of course, a final class that has a package-private getter to a
>>>>>> shared
>>>>>> copy of its backing array could be considered to be sending mixed
>>>>>> messages...
>>>>>>
>>>>>> It is possible that a mutable class might have significant performance
>>>>>> advantages over an immutable one beyond saving a few bytes. For
>>>>>> example,
>>>>>> if
>>>>>> the updates are simple, and depend on the previous value of the cell,
>>>>>> then
>>>>>> a mutable version might have better cache behavior. If there's other
>>>>>> sources of cache pressure this might have a higher than expected
>>>>>> impact.
>>>>>> The costs of copying the original values might also be relatively
>>>>>> significant.
>>>>>>
>>>>>> For an ETL use case these issues are unlikely to be limiting factors;
>>>>>> for a
>>>>>> start, there's a non-zero chance that a CSVRecord was extracted by
>>>>>> parsing a CSV file. Also a transform will require conversion to some
>>>>>> sort
>>>>>> of Number (or String allocation).
>>>>>>
>>>>>> The current API doesn't easily support adding alternate
>>>>>> implementations
>>>>>> of
>>>>>> the relevant types. Implementation classes are final, and important
>>>>>> return
>>>>>> types are concrete.
>>>>>>
>>>>>> One solution might be to treat the current code as almost an
>>>>>> implementation
>>>>>> module, define a separate API module, and add extra interfaces and
>>>>>> alternate implementations to support the target use case (mutable
>>>>>> records, streams, reactivex, transform functions or what have you).
>>>>>>
>>>>>> Simon
>>>>>>
>>>>>>
>>>>>>
>>>>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
> For additional commands, e-mail: dev-h...@commons.apache.org
>
>
