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.
