Mailing-List: contact dev-help@commons.apache.org; run by ezmlm
Precedence: bulk
Reply-To: "Commons Developers List" <dev@commons.apache.org>
MIME-Version: 1.0
In-Reply-To: <CAKQZcJrfQyNMt5H69ytN6buwVdOUf_PxPFo6v0pq7cVj7syOrg@mail.gmail.com>
References: <CAKQZcJofDF9ekQtL2R77ZdSmgPcUBX4pAaZXw4HqAd7nf7aocA@mail.gmail.com>
 <CACZkXPycicBfOOqkPSo6jqfm=o+X89c+UKy_XzcvEWiMcY9ivg@mail.gmail.com>
 <CAKQZcJqjdgiCgBqyLsL=jzDi9k4PqOmt7nZ6eataKJe1ZQUSdA@mail.gmail.com>
 <CACZkXPx+gD=5_mNtA04csP9RD6PzHv7Y1R9T3GwekmjMaDxfCw@mail.gmail.com>
 <CACZkXPxOpkb1Wzyooi4dRZEQJbY+hJf=W8YETFva8C7CJJ=3aw@mail.gmail.com>
 <C7CFFB1B-DC56-418C-991E-08D7C0D6C052@apache.org> <CACZkXPwDfj-Kaimtu9Pqd1_cFsVs57+GyCneSfDR-SBtnuTNXw@mail.gmail.com>
 <0f45296071845faf757344a56b7e2ef0@scarlet.be> <CACZkXPz3YKGFt7fHa26Syjwx90Gf5qXQWZqEoZH7zH6zFKRX5g@mail.gmail.com>
 <6d3f648f999cd9ff726040b5ce84dc64@scarlet.be> <CAKQZcJqgaoNDZmCnNAzs0nEmvQo97MjnF7p0=wU189TeYqryvQ@mail.gmail.com>
 <CACZkXPwdk4_A79=VEjupKbDSyQeXoiaJ_Gh=Q-DKjJyiv+r3jw@mail.gmail.com>
 <372b12c62d332dbbd71f1889d79855a1@scarlet.be> <CAKQZcJpHxs90VvZ2o+tKmmyd21OQdmcb97kMo++aRMj5UE=VFQ@mail.gmail.com>
 <6d54922767856921282813460cbfc106@scarlet.be> <CACZkXPy1OBjb+VkYRt_mKXZ9DO+pyugCdBhuDVyC+o_Uj-VFgQ@mail.gmail.com>
 <CAKQZcJrfQyNMt5H69ytN6buwVdOUf_PxPFo6v0pq7cVj7syOrg@mail.gmail.com>
From: Gary Gregory <garydgregory@gmail.com>
Date: Thu, 17 Aug 2017 11:57:57 -0600
Message-ID: <CACZkXPwDX5kq_AbOe1TUq6w-jnGd5BwrN68CLE3Sb-UiA+aUvQ@mail.gmail.com>
Subject: Re: [CSV] CSVMutableRecord
To: Commons Users List <user@commons.apache.org>
Cc: Commons Developers List <dev@commons.apache.org>
Content-Type: multipart/alternative; boundary="001a11441148ac45820556f6c021"
archived-at: Thu, 17 Aug 2017 17:58:05 -0000

--001a11441148ac45820556f6c021
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

Not yet ;-)

On Aug 17, 2017 11:34, "nitin mahendru" <nitin.mahendru88@gmail.com> wrote:

> Hi All,
>
> Any consensus on this ?
>
> -Nitin
>
>
>
>
> On Tue, Aug 15, 2017 at 4:43 PM Gary Gregory <garydgregory@gmail.com>
> wrote:
>
> > On Tue, Aug 15, 2017 at 5:32 PM, Gilles <gilles@harfang.homelinux.org>
> > wrote:
> >
> > > On Tue, 15 Aug 2017 22:52:32 +0000, nitin mahendru wrote:
> > >
> > >> On Tue, 15 Aug 2017 12:02:20 -0600, Gary Gregory wrote:
> > >>
> > >>> On Tue, Aug 15, 2017 at 10:38 AM, nitin mahendru
> > >>> <nitin.mahendru88@gmail.com
> > >>>
> > >>>> wrote:
> > >>>>
> > >>>
> > >>> How about having a state in the class itself which says that it's
> > >>>> mutable
> > >>>> or not.
> > >>>> If we call a setter on an immutable then it throws an exception.
> > >>>> By default the records are immutable and you need to make them
> > >>>> mutable
> > >>>> using a new API.
> > >>>>
> > >>>
> > >> A code example would be useful...
> > >>
> > >>
> > >>
> > >>
> > >> Below is the pull request I added.
> > >> https://github.com/apache/commons-csv/pull/21
> > >>
> > >
> > > As I indicated in the previous message, this is functionally
> > > breaking. [I'm diverting this discussion over to the "dev"
> > > mailing list.]
> > >
> >
> > 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.
> >
> > Gary
> >
> >
> > >
> > > The following should be an interesting read:
> > >   http://markmail.org/message/6ytvmxvy2ndsfp7h
> > >
> > >
> > > Regards,
> > > Gilles
> > >
> > >
> > >
> > >
> > >> On Tue, Aug 15, 2017 at 11:17 AM Gilles <gilles@harfang.homelinux.or=
g
> >
> > >> wrote:
> > >>
> > >> On Tue, 15 Aug 2017 12:02:20 -0600, Gary Gregory wrote:
> > >>> > On Tue, Aug 15, 2017 at 10:38 AM, nitin mahendru
> > >>> > <nitin.mahendru88@gmail.com
> > >>> >> wrote:
> > >>> >
> > >>> >> How about having a state in the class itself which says that it'=
s
> > >>> >> mutable
> > >>> >> or not.
> > >>> >> If we call a setter on an immutable then it throws an exception.
> > >>> >> By default the records are immutable and you need to make them
> > >>> >> mutable
> > >>> >> using a new API.
> > >>>
> > >>> A code example would be useful...
> > >>>
> > >>> >> pros: Saves memory, Keeps the immutability benefits
> > >>>
> > >>> What kind of usage are you considering that a single transient
> > >>> record matters (as compared to the ~300 MB of the JVM itself...)?
> > >>>
> > >>> >> cons: people using "mutable" records need to be careful.(While
> > >>> >> threading
> > >>> >> maybe)
> > >>> >>
> > >>> >
> > >>> > Interesting idea!
> > >>> >
> > >>> > But I think I like the idea of a subclass better if we are going =
to
> > >>> > split
> > >>> > the behavior b/w mutable and immutable.
> > >>>
> > >>> Once you have a subclass that is able to modify the state of
> > >>> its parent, it's a mutable object. Period.
> > >>> There is no such thing as a "split".
> > >>>
> > >>> >
> > >>> > For my money and the KISS principle, I would just add the put
> method
> > >>> > in
> > >>> > CSVRecord.
> > >>>
> > >>> Then, any use that assumes immutability will be broken.
> > >>>
> > >>>
> > >>> Gilles
> > >>>
> > >>>
> > >>> > Gary
> > >>> >
> > >>> >>
> > >>> >> -Nitin
> > >>> >>
> > >>> >>
> > >>> >>
> > >>> >>
> > >>> >> On Tue, Aug 15, 2017 at 9:01 AM Gilles
> > >>> >> <gilles@harfang.homelinux.org>
> > >>> >> wrote:
> > >>> >>
> > >>> >> > On Tue, 15 Aug 2017 09:49:04 -0600, Gary Gregory wrote:
> > >>> >> > > That looks odd to me. What comes up for me is the use case
> where
> > >>> >> I
> > >>> >> > > want to
> > >>> >> > > ETL a file of 10,000,000 records and update, say, one column=
.
> If
> > >>> >> am
> > >>> >> > > forced
> > >>> >> > > to create a brand new record for every record read, that wou=
ld
> > >>> >> be a
> > >>> >> > > shame.
> > >>> >> >
> > >>> >> > Why?
> > >>> >> >
> > >>> >> > > If I had a mutable record, I could just keep on updating it
> and
> > >>> >> using
> > >>> >> > > it to
> > >>> >> > > write each row. Read record, update it, write record. No ext=
ra
> > >>> >> memory
> > >>> >> > > needed.
> > >>> >> >
> > >>> >> > How is the size of 1 additional record going to matter compare=
d
> to
> > >>> >> the
> > >>> >> > size of the whole program?
> > >>> >> >
> > >>> >> > > Either we can make the current record mutable (what's the
> harm?)
> > >>> >> or
> > >>> >> > > we can
> > >>> >> > > make the parser serve out mutable records based on a config
> > >>> >> setting.
> > >>> >> > > This
> > >>> >> > > could be a subclass of CSVRecord with the extra method I
> > >>> >> proposed.
> > >>> >> >
> > >>> >> > The harm is that you loose all the promises of immutability.
> > >>> >> >
> > >>> >> > Regards,
> > >>> >> > Gilles
> > >>> >> >
> > >>> >> > >
> > >>> >> > > Thoughts?
> > >>> >> > >
> > >>> >> > > Gary
> > >>> >> > >
> > >>> >> > > On Tue, Aug 15, 2017 at 8:33 AM, Gilles
> > >>> >> > > <gilles@harfang.homelinux.org>
> > >>> >> > > wrote:
> > >>> >> > >
> > >>> >> > >> On Tue, 15 Aug 2017 08:01:53 -0600, Gary Gregory wrote:
> > >>> >> > >>
> > >>> >> > >>> How does that work when you want to change more than one
> > >>> >> value?
> > >>> >> > >>>
> > >>> >> > >>
> > >>> >> > >> How about a "vararg" argument:
> > >>> >> > >>
> > >>> >> > >> /**
> > >>> >> > >>  * @param orig Original to be copied.
> > >>> >> > >>  * @param replace Fields to be replaced.
> > >>> >> > >>  */
> > >>> >> > >> public static CSVRecord createRecord(CSVRecord orig,
> > >>> >> > >>                                      Pair<Integer, String>
> ...
> > >>> >> > >> replace) {
> > >>> >> > >>     // ...
> > >>> >> > >> }
> > >>> >> > >>
> > >>> >> > >>
> > >>> >> > >> Gilles
> > >>> >> > >>
> > >>> >> > >>
> > >>> >> > >>
> > >>> >> > >>> Gary
> > >>> >> > >>>
> > >>> >> > >>> On Aug 15, 2017 00:17, "Benedikt Ritter" <
> britter@apache.org>
> > >>> >> > >>> wrote:
> > >>> >> > >>>
> > >>> >> > >>> Hi,
> > >>> >> > >>>>
> > >>> >> > >>>> I very much like that CSVRecord is unmodifiable. So I=E2=
=80=99d
> > >>> >> suggest an
> > >>> >> > >>>> API,
> > >>> >> > >>>> that creates a new record instead of mutating the existin=
g
> > >>> >> one:
> > >>> >> > >>>>
> > >>> >> > >>>> CSVRecord newRecord =3D myRecord.put(1, =E2=80=9Evalue")
> > >>> >> > >>>>
> > >>> >> > >>>> I=E2=80=99m not sure about =E2=80=9Eput=E2=80=9C as a met=
hod name since it clashes
> > >>> >> with
> > >>> >> > >>>> java.util.Map#put, which is mutation based...
> > >>> >> > >>>>
> > >>> >> > >>>> Regards,
> > >>> >> > >>>> Benedikt
> > >>> >> > >>>>
> > >>> >> > >>>> > Am 15.08.2017 um 02:54 schrieb Gary Gregory
> > >>> >> > >>>> <garydgregory@gmail.com>:
> > >>> >> > >>>> >
> > >>> >> > >>>> > Feel free to provide a PR on GitHub :-)
> > >>> >> > >>>> >
> > >>> >> > >>>> > Gary
> > >>> >> > >>>> >
> > >>> >> > >>>> > On Aug 14, 2017 15:29, "Gary Gregory"
> > >>> >> <garydgregory@gmail.com>
> > >>> >> > >>>> wrote:
> > >>> >> > >>>> >
> > >>> >> > >>>> >> I think we've kept the design as YAGNI as possible...
> :-)
> > >>> >> > >>>> >>
> > >>> >> > >>>> >> Gary
> > >>> >> > >>>> >>
> > >>> >> > >>>> >> On Mon, Aug 14, 2017 at 3:25 PM, nitin mahendru <
> > >>> >> > >>>> >> nitin.mahendru88@gmail.com> wrote:
> > >>> >> > >>>> >>
> > >>> >> > >>>> >>> Yeah that also is OK. I though there is a reason to
> keep
> > >>> >> the
> > >>> >> > >>>> CSVRecord
> > >>> >> > >>>> >>> without setters. But maybe not!
> > >>> >> > >>>> >>>
> > >>> >> > >>>> >>> Nitin
> > >>> >> > >>>> >>>
> > >>> >> > >>>> >>>
> > >>> >> > >>>> >>>
> > >>> >> > >>>> >>>
> > >>> >> > >>>> >>> On Mon, Aug 14, 2017 at 2:22 PM Gary Gregory
> > >>> >> > >>>> <garydgregory@gmail.com
> > >>> >> > >>>> >
> > >>> >> > >>>> >>> wrote:
> > >>> >> > >>>> >>>
> > >>> >> > >>>> >>>> Hi All:
> > >>> >> > >>>> >>>>
> > >>> >> > >>>> >>>> Should we consider adding put(int,Object) and
> > >>> >> put(String,
> > >>> >> > >>>> Object) to
> > >>> >> > >>>> the
> > >>> >> > >>>> >>>> current CSVRecord class?
> > >>> >> > >>>> >>>>
> > >>> >> > >>>> >>>> Gary
> > >>> >> > >>>> >>>>
> > >>> >> > >>>> >>>> On Mon, Aug 14, 2017 at 2:54 PM, nitin mahendru <
> > >>> >> > >>>> >>>> nitin.mahendru88@gmail.com>
> > >>> >> > >>>> >>>> wrote:
> > >>> >> > >>>> >>>>
> > >>> >> > >>>> >>>>> Hi Everyone,
> > >>> >> > >>>> >>>>>
> > >>> >> > >>>> >>>>> I recently pushed a change(pull request 20) to get
> the
> > >>> >> line
> > >>> >> > >>>> ending
> > >>> >> > >>>> >>> from
> > >>> >> > >>>> >>>> the
> > >>> >> > >>>> >>>>> parser.
> > >>> >> > >>>> >>>>>
> > >>> >> > >>>> >>>>> Now I want to push another change which I feel will
> > >>> >> also be
> > >>> >> > >>>> useful
> > >>> >> > >>>> for
> > >>> >> > >>>> >>>> the
> > >>> >> > >>>> >>>>> community. I want to add a CSVRecordMutable class
> which
> > >>> >> had
> > >>> >> > >>>> a
> > >>> >> > >>>> >>> constructor
> > >>> >> > >>>> >>>>> which accepts a CSVRecord object. So when we have a
> > >>> >> > >>>> CSVRecordMutable
> > >>> >> > >>>> >>>> object
> > >>> >> > >>>> >>>>> from it then we can edit individual columns using i=
t.
> > >>> >> > >>>> >>>>>
> > >>> >> > >>>> >>>>> I would be using this to write back my edited CSV
> file.
> > >>> >> My
> > >>> >> > >>>> use case
> > >>> >> > >>>> >>> is to
> > >>> >> > >>>> >>>>> read a csv, mangle some columns, write back a new
> csv.
> > >>> >> > >>>> >>>>>
> > >>> >> > >>>> >>>>> I could have directly raised a pull request but I
> just
> > >>> >> > >>>> wanted to
> > >>> >> > >>>> float
> > >>> >> > >>>> >>>> the
> > >>> >> > >>>> >>>>> idea before and see the reaction.
> > >>> >> > >>>> >>>>>
> > >>> >> > >>>> >>>>> Thanks
> > >>> >> > >>>> >>>>>
> > >>> >> > >>>> >>>>> Nitin
> > >>> >> > >>>> >>>>>
> > >>> >> > >>>> >>>>
> > >>> >> > >>>> >>>
> > >>> >> > >>>> >>
> > >>> >> > >>>> >>
> > >>> >> > >>>>
> > >>> >> > >>>>
> > >>> >> > >>>>
> > >>> >> > >>
> > >>>
> > >>>
> > >>>
> > >>> ------------------------------------------------------------
> ---------
> > >>> To unsubscribe, e-mail: user-unsubscribe@commons.apache.org
> > >>> For additional commands, e-mail: user-help@commons.apache.org
> > >>>
> > >>>
> > >>>
> > >
> > > ---------------------------------------------------------------------
> > > To unsubscribe, e-mail: user-unsubscribe@commons.apache.org
> > > For additional commands, e-mail: user-help@commons.apache.org
> > >
> > >
> >
>

--001a11441148ac45820556f6c021--