On Fri, Aug 18, 2017 at 11:52 AM, Gilles <gil...@harfang.homelinux.org> 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). > > In any cases, a mutable subclass is not a solution IMO. > > Not sure I'm clear enough... :-) hm, so add the 2 put APIs and call it 2.0. That's fine with me. Gary > > 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 > >
