Personally, I would like to see CSV record made mutable. You could get one from the parser, make a few changes to it, then pass it to the printer.

-Adrian


Quoting Gary Gregory <garydgreg...@gmail.com>:

On Wed, Jan 22, 2014 at 11: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?


That the heart of the matter indeed.

For the sake of getting 1.0 out the door, we can leave it read-only.

If we want to make it read-write, we can do that later, in 1.1 or 2.0.
Whether or not this is done while preserving BC will be a different matter.

Gary




-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




--
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

Reply via email to