RFR: 6206780 (str) Forwarding append methods in String{Buffer, Builder} are inconsistent

Chris Hegarty chris.hegarty at oracle.com
Wed Sep 26 10:12:08 UTC 2012


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.

  - Trivially, the year in the header of the tests is incorrect

  - 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.

-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
>



More information about the core-libs-dev mailing list