Add getChars to CharSequence

Mike Duigou mike.duigou at oracle.com
Thu May 9 00:30:56 UTC 2013


Hi Martin;

Some notes from a non-exhaustive (ran out of time before dinner) review.

Mike

AbstractStringBuilder::

- The impls like insert(int dstOffset, CharSequence s) makes me slightly uneasy because the private value field escapes to an unknown class. Who knows what evil (or foolishness) could result?


Direct-X-Buffer.java::

- +#if[rw] public boolean isDirect() : Why would this be conditionalized with rw?


Heap-X-Buffer.java::

- protected -> private int ix(int i) : Is Alan OK with this change. I've mostly avoided these templates. :-)


X-Buffer.java.template::

- toString() could use the JavaLangAccess.newUnsafeString() constructor!

- I prefer your formatting of "return bigEndian ?".


test/.../GetChars::

- Great to see you've already adopted using TestNG for JTReg tests!

- ArraysCharSequence.hashCode() could have been Arrays.hashcode(chars) or not implemented at all.

- Could use a @DataProivder for "CharSequence[] seqs = {" style tests.

- There's been some informal discussion of packaging commonly used test utils as jtreg @library. This could be a good candidate. ArraysCharSequence is a candidate for testing utils lib. Basic impls that override no defaults are going to be increasingly important for testing.

- I like your varargs assertsThrows. I have written a non-varargs one in test/.../Map/Defaults.java. Your varargs one of necessity violates the actual, expected, [message] pattern used by other TestNG assertions but I prefer it. This is also a candidate for utils lib.

On May 1 2013, at 15:19 , Martin Buchholz wrote:

> Another version of this change is ready.  No longer preliminary.  Tests
> have been written.
> http://cr.openjdk.java.net/~martin/webrevs/openjdk8/getChars/
> 
> This kind of change is particularly subject to feature creep, and I am
> trying to restrain myself.
> 
> I even addressed some of Ulf's suggestions.
> The "Msg" suffix  is gone.
> I reverted changes to AbstractStringBuilder.replace.
> I kept the naming convention for getChars parameter names.
> Parameter names and exception details continue to be maddeningly
> unsystematic, but they should be a little better than before.




More information about the core-libs-dev mailing list