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