commons-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Gilles <gil...@harfang.homelinux.org>
Subject Re: [CSV] CSVMutableRecord
Date Fri, 18 Aug 2017 21:38:20 GMT
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.

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
View raw message