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] CSVMutableRecord
Date Fri, 18 Aug 2017 21:46:11 GMT
On Fri, Aug 18, 2017 at 3:38 PM, Gilles <gilles@harfang.homelinux.org>
wrote:

> On Fri, 18 Aug 2017 13:21:45 -0600, Gary Gregory wrote:
>
>> On Fri, Aug 18, 2017 at 12:50 PM, Gilles <gilles@harfang.homelinux.org>
>> wrote:
>>
>> On Fri, 18 Aug 2017 12:41:01 -0600, Gary Gregory wrote:
>>>
>>> On Wed, Aug 16, 2017 at 6:28 PM, Gilles <gilles@harfang.homelinux.org>
>>>> wrote:
>>>>
>>>> On Wed, 16 Aug 2017 11:27:53 -0600, Gary Gregory wrote:
>>>>
>>>>>
>>>>> Let's summarize the options:
>>>>>
>>>>>>
>>>>>> 0) do nothing
>>>>>> 1) Add two put methods to CVSRecord making the class mutable
>>>>>> 2) Add a "mutableRecord" boolean option to CVSRecord and CSVFormat
>>>>>> such
>>>>>> that a new boolean in CVSRecord allow method from 1) above to either
>>>>>> work
>>>>>> or throw an exception.
>>>>>> 3) Add a "mutableRecord" boolean option to CVSRecord and CSVFormat
>>>>>> such
>>>>>> that subclass of CVSRecord called CVSMutableRecord is created which
>>>>>> contains two new put methods
>>>>>>
>>>>>> What else?
>>>>>>
>>>>>>
>>>>>> 4) The factory method:
>>>>>  /**
>>>>>   * @param orig Original to be copied.
>>>>>   * @param replace Fields to be replaced.
>>>>>   * @return a copy of "orig", except for the fields in "replace".
>>>>>   */
>>>>>  public static CSVRecord createRecord(CSVRecord orig,
>>>>>                                       Pair<Integer, String> ...
>>>>> replace)
>>>>>
>>>>> Could also be:
>>>>>  public static CSVRecord createRecord(CSVRecord orig,
>>>>>                                       int[] replaceIndices,
>>>>>                                       String[] replaceValues)
>>>>>
>>>>>
>>>>
>>>> To me, that feels like bending over backwards and hard to very ugly to
>>>> use,
>>>> building an array of ints, building an array of Strings. Yikes. But, hey
>>>> that's just me.
>>>>
>>>>
>>> What about the "Pair" alternative above?
>>>
>>> Another alternative would be to have a "CSVRecordBuilder" (with "put"
>>> methods):
>>> ---CUT---
>>> CSVRecord rec = readRecord(source);
>>> rec = new CSVRecordBuilder(rec).put(1, "Change").put(4, "this").build();
>>> ---CUT---
>>>
>>>
>> For my money, the above is convoluted for no reason from a user's POV,
>>
>
> I surely agree that the "builder" approach is less obvious
> than adding methods as the need seems to arise.
> But the part "for no reason" is simply not true.
>
> My opinion is that a library is more useful if it limits
> the number of ways one can shoot one's foot. YMMV. :-)
>
> What are the consequences in various use-cases?
> Is mutability or immutability useful in selected cases?
> And from there, how to modify the design to gracefully
> handle all of them?
>
> Assuming that you want to define a mutable class for
> some of those cases, it might make sense to provide
> a factory to create an immutable instance:
>
> public class CSVRecord {
>
>   /** Deep copy constructor. */
>   public CSVRecord(CSVRecord rec) { /* ... */ }
>
>   public void put(int i, String s) { /* ... */ }
>
>   /** Create an immutable copy. */
>   public CSVRecord createImmutable() {
>     return new CSVRecord(this) {
>       public void put(int i, String s) {
>         throw new UnsupportedOperationException();
>       }
>     }
>   }
> }
>
> That way, in the new release, users that relied on the
> immutable character of "CSVRecord" can at least modify
> their code at the (hopefully few) right places and still
> maintain consistency.


I did something like that in the branch CSV-216 but that tweaks a lot of
code and not every one liked that.

Gary

>
>
> Gilles
>
> again compared to the simple:
>>
>> rec.put(1, "Change");
>> rec.put(4, "this");
>>
>> Gary
>>
>>
>>>
>>>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> For additional commands, e-mail: dev-help@commons.apache.org
>
>

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