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... :-)
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