On Fri, 18 Aug 2017 15:46:11 -0600, Gary Gregory wrote:
On Fri, Aug 18, 2017 at 3:38 PM, Gilles <gil...@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 <gil...@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 <gil...@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.

Perhaps because "mutable or not" is not part of the format...

Here the immutability is provided on a per-record basis.
Anyways, I'm not even sure that it's a good enough substitute
for compiler-enforced immutability. :-{

Finally, it boils down to what exactly the class "CSVRecord"
represents.  Your view is definitely different from the
original designers' (perhaps a younger you was among them?).

Gilles

Gary



Gilles

again compared to the simple:

rec.put(1, "Change");
rec.put(4, "this");

Gary







---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
For additional commands, e-mail: dev-h...@commons.apache.org

Reply via email to