On Wed, Jan 22, 2014 at 11:27 AM, James Ring <s...@jdns.org> wrote:
> There is nothing that says Map must be mutable, even the javadoc says:
>
> "put - Associates the specified value with the specified key in this
> map (optional operation)."
>
Hm, good point, so we could have CSVRecord implement Map but throw
exceptions for write-ops (for now).
Gary
> On Wed, Jan 22, 2014 at 8:03 AM, <adrian.c...@sandglass-software.com>
> wrote:
> > There is no question your use case is convenient, but it violates the
> > contract that a CSV record is read-only. Calling put() on a CSV record
> is a
> > mutating operation.
> >
> > So, is a CSV record read-only or not?
> >
> >
> > -Adrian
> >
> > Quoting Gary Gregory <garydgreg...@gmail.com>:
> >
> >> On Wed, Jan 22, 2014 at 10:04 AM,
> >> <adrian.c...@sandglass-software.com>wrote:
> >>
> >>> I disagree. If the CSV record is read-only, then the List and Map
> >>> representations of the CSV record should be read-only too. Making those
> >>> representations writable is confusing - it gives the impression you are
> >>> modifying the CSV record when in fact you are not.
> >>>
> >>> I don't see how it restricts their usability.
> >>>
> >>
> >> I guess I should have started with another use-case: take a CSV, remove
> a
> >> column, and compute a new column.
> >>
> >> Right now I can do (to try, add this to CSVRecordTest):
> >>
> >> @Test
> >> public void testRemoveAndAddColumns() throws IOException {
> >> // do:
> >> final CSVPrinter printer = new CSVPrinter(new StringBuilder(),
> >> CSVFormat.DEFAULT);
> >> final Map<String, String> map = recordWithHeader.toMap();
> >> map.remove("OldColumn");
> >> map.put("NewColumn", "NewValue");
> >> // check:
> >> final ArrayList<String> list = new
> >> ArrayList<String>(map.values());
> >> Collections.sort(list);
> >> printer.printRecord(list);
> >> Assert.assertEquals("A,B,C,NewValue" +
> >> CSVFormat.DEFAULT.getRecordSeparator(), printer.getOut().toString());
> >> }
> >>
> >> I would have to create a wasteful temp Map if the Map I get from the
> >> record
> >> is read-only. This code is bad enough.
> >>
> >> If the record is a Map, there is no song and dance at all:
> >>
> >> // do:
> >> final CSVPrinter printer = new CSVPrinter(new StringBuilder(),
> >> CSVFormat.DEFAULT);
> >> recordWithHeader.remove("OldColumn");
> >> recordWithHeader.put("NewColumn", "NewValue");
> >> // check:
> >> printer.printRecord(recordWithHeader);
> >> Assert.assertEquals("A,B,C,NewValue" +
> >> CSVFormat.DEFAULT.getRecordSeparator(), printer.getOut().toString());
> >>
> >>
> >> If you want a special kind of Map (a blinking read-only map ;), you can
> >> call putIn(Map).
> >>
> >> Gary
> >>
> >>>
> >>> -Adrian
> >>>
> >>> Quoting Gary Gregory <garydgreg...@gmail.com>:
> >>>
> >>> On Wed, Jan 22, 2014 at 9:40 AM, <adrian.c...@sandglass-software.com>
> >>>>
> >>>> wrote:
> >>>>
> >>>> CORRECTION: The patch in the Jira issue is a simplified version of
> >>>> Gary's
> >>>>>
> >>>>> implementation.
> >>>>>
> >>>>> I started with the design I proposed - an inner class Map
> >>>>> implementation
> >>>>> backed by CSVRecord, but then implementing entrySet() and such became
> >>>>> complicated. So I changed it to the inner class being backed by a
> >>>>> HashMap,
> >>>>> then reduced that to a simplified version of Gray's implementation.
> >>>>>
> >>>>> I apologize for the confusion.
> >>>>>
> >>>>>
> >>>> I do not see the point of making the new objects (list, map)
> read-only.
> >>>> Since the objects are not connected to the record, there should be no
> >>>> expectancy of synchronizing the map with the record. There are "to"
> >>>> methods
> >>>> after all. This just restricts the usability of the objects.
> >>>>
> >>>> Gary
> >>>>
> >>>>
> >>>>> -Adrian
> >>>>>
> >>>>>
> >>>>> Quoting Adrian Crum <adrian.c...@sandglass-software.com>:
> >>>>>
> >>>>> I agree with Emmanuel. We don't want CSVRecord to devolve into a god
> >>>>>
> >>>>>> class that tries to be everything to everybody. "Do only one thing,
> >>>>>> and
> >>>>>> do
> >>>>>> it really well" is the design pattern I prefer.
> >>>>>>
> >>>>>> I created a patch for the design I proposed a few days ago. Please
> >>>>>> consider using it.
> >>>>>>
> >>>>>> https://issues.apache.org/jira/browse/CSV-104
> >>>>>>
> >>>>>>
> >>>>>> Adrian Crum
> >>>>>> Sandglass Software
> >>>>>> www.sandglass-software.com
> >>>>>>
> >>>>>> On 1/22/2014 8:01 AM, Emmanuel Bourg wrote:
> >>>>>>
> >>>>>> Le 21/01/2014 15:08, Gary Gregory a écrit :
> >>>>>>>
> >>>>>>>
> >>>>>>> This kind of complication is unnecessary IMO, why not just have
> the
> >>>>>>>
> >>>>>>>> record
> >>>>>>>> implement Map?
> >>>>>>>>
> >>>>>>>>
> >>>>>>> Because a record is neither a List nor a Map.
> >>>>>>>
> >>>>>>> A map decorator isn't that complicated, and the work has already
> been
> >>>>>>> done in [configuration]. It would be trivial to adapt the code to
> >>>>>>> [csv].
> >>>>>>>
> >>>>>>> Emmanuel Bourg
> >>>>>>>
> >>>>>>>
> >>>>>>>
> ---------------------------------------------------------------------
> >>>>>>> To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
> >>>>>>> For additional commands, e-mail: dev-h...@commons.apache.org
> >>>>>>>
> >>>>>>>
> >>>>>>>
> >>>>>>>
> ---------------------------------------------------------------------
> >>>>>>
> >>>>>> To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
> >>>>>> For additional commands, e-mail: dev-h...@commons.apache.org
> >>>>>>
> >>>>>>
> >>>>>>
> >>>>>>
> >>>>>
> >>>>>
> >>>>> ---------------------------------------------------------------------
> >>>>> To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
> >>>>> For additional commands, e-mail: dev-h...@commons.apache.org
> >>>>>
> >>>>>
> >>>>>
> >>>>
> >>>> --
> >>>> E-Mail: garydgreg...@gmail.com | ggreg...@apache.org
> >>>> Java Persistence with Hibernate, Second Edition<http://www.manning.
> >>>> com/bauer3/>
> >>>> JUnit in Action, Second Edition <http://www.manning.com/tahchiev/>
> >>>> Spring Batch in Action <http://www.manning.com/templier/>
> >>>>
> >>>> Blog: http://garygregory.wordpress.com
> >>>> Home: http://garygregory.com/
> >>>> Tweet! http://twitter.com/GaryGregory
> >>>>
> >>>>
> >>>
> >>>
> >>>
> >>> ---------------------------------------------------------------------
> >>> To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
> >>> For additional commands, e-mail: dev-h...@commons.apache.org
> >>>
> >>>
> >>
> >>
> >> --
> >> E-Mail: garydgreg...@gmail.com | ggreg...@apache.org
> >> Java Persistence with Hibernate, Second
> >> Edition<http://www.manning.com/bauer3/>
> >> JUnit in Action, Second Edition <http://www.manning.com/tahchiev/>
> >> Spring Batch in Action <http://www.manning.com/templier/>
> >> Blog: http://garygregory.wordpress.com
> >> Home: http://garygregory.com/
> >> Tweet! http://twitter.com/GaryGregory
> >>
> >
> >
> >
> >
> > ---------------------------------------------------------------------
> > To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
> > For additional commands, e-mail: dev-h...@commons.apache.org
> >
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
> For additional commands, e-mail: dev-h...@commons.apache.org
>
>
--
E-Mail: garydgreg...@gmail.com | ggreg...@apache.org
Java Persistence with Hibernate, Second Edition<http://www.manning.com/bauer3/>
JUnit in Action, Second Edition <http://www.manning.com/tahchiev/>
Spring Batch in Action <http://www.manning.com/templier/>
Blog: http://garygregory.wordpress.com
Home: http://garygregory.com/
Tweet! http://twitter.com/GaryGregory
