On 09/26/2012 06:12 AM, Chris Hegarty wrote:
Hi Jim,
This is a nice cleanup, and certainly makes builder and buffer more
consistent.
Some specific comments:
- StringBuilder
Why the change to the public doc for append(StringBuffer)? It
looks wrong to me. It is now taking about the wrong given type. \
accident - fixed.
- Trivially, the year in the header of the tests is incorrect
Fixed
- StringBuffer
I don't see that any of the new synchronized blocks are necessary.
If ASB is doing exactly the same as the previous buffer code, then
it will invoke the overridden subclass methods that are already
synchronized.
Good catch. I verified this, and you're absolutely right. Fixed.
Thanks. I'll re-spin the patch and send it your way for pushing, if you
would be so kind, please.
Cheers,
Jim
-Chris.
On 25/09/2012 23:03, Jim Gish wrote:
Please review the change at
http://cr.openjdk.java.net/~jgish/Bug6206780-sb-append-forward
Overview --
* introduced consistent forwarding to AbstractStringBuilder from
StringBuffer and StringBuilder for append and other methods per the
bug report.
* Added missing @Override annotations.
* Got rid of knowledge of the sub-classes from ASB (smelled too bad to
leave in).
* Retained all methods to maintain compatibility (even though some
like append(StringBuffer) could go away because append(CharSequence)
would do).
* To further maintain compatibility, used sychronized(this) in place
of adding synchronized methods to those methods in StringBuffer that
now need it (because of type-based dispatch being done in super
rather than in /both/ StringBuffer and StringBuilder with
CharSequence args).
* Ensured that StringBuffer.append(StringBuilder) and
StringBuilder.append(StringBuffer) are both handled properly.
Thanks,
Jim
--
Jim Gish | Consulting Member of Technical Staff | +1.781.442.0304
Oracle Java Platform Group | Core Libraries Team
35 Network Drive
Burlington, MA 01803
jim.g...@oracle.com
