commons-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From adrian.c...@sandglass-software.com
Subject Re: [csv] CSVRecord and Map<String, String>
Date Wed, 22 Jan 2014 16:30:24 GMT
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 <garydgregory@gmail.com>:

> On Wed, Jan 22, 2014 at 11:03 AM, <adrian.crum@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 <garydgregory@gmail.com>:
>>
>>  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
>>>
>>>
>>
>>
>>
>> ---------------------------------------------------------------------
>> 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


Mime
View raw message