commons-user mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From nitin mahendru <nitin.mahendr...@gmail.com>
Subject Re: [CSV] CSVMutableRecord
Date Tue, 15 Aug 2017 22:52:32 GMT
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




On Tue, Aug 15, 2017 at 11:17 AM Gilles <gilles@harfang.homelinux.org>
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 would
> >> 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 extra
> >> memory
> >> > > needed.
> >> >
> >> > How is the size of 1 additional record going to matter compared 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’d
> >> suggest an
> >> > >>>> API,
> >> > >>>> that creates a new record instead of mutating the existing
> >> one:
> >> > >>>>
> >> > >>>> CSVRecord newRecord = myRecord.put(1, „value")
> >> > >>>>
> >> > >>>> I’m not sure about „put“ as a method 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 it.
> >> > >>>> >>>>>
> >> > >>>> >>>>> 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
>
>

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message