commons-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Gary Gregory <garydgreg...@gmail.com>
Subject Re: [csv] CSVRecord and Map<String, String>
Date Wed, 22 Jan 2014 15:42:06 GMT
On Wed, Jan 22, 2014 at 10:04 AM, <adrian.crum@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 <garydgregory@gmail.com>:
>
>  On Wed, Jan 22, 2014 at 9:40 AM, <adrian.crum@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.crum@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-unsubscribe@commons.apache.org
>>>>> For additional commands, e-mail: dev-help@commons.apache.org
>>>>>
>>>>>
>>>>>  ---------------------------------------------------------------------
>>>> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
>>>> For additional commands, e-mail: dev-help@commons.apache.org
>>>>
>>>>
>>>>
>>>>
>>>
>>>
>>> ---------------------------------------------------------------------
>>> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
>>> For additional commands, e-mail: dev-help@commons.apache.org
>>>
>>>
>>>
>>
>> --
>> E-Mail: garydgregory@gmail.com | ggregory@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-unsubscribe@commons.apache.org
> For additional commands, e-mail: dev-help@commons.apache.org
>
>


-- 
E-Mail: garydgregory@gmail.com | ggregory@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

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