On 06/10/2012 01:52, Jim Gish wrote:
I've revamped the previous proposed change. Please review the update
at http://cr.openjdk.java.net/~jgish/Bug6206780-sb-append-forward/
<http://cr.openjdk.java.net/%7Ejgish/Bug6206780-sb-append-forward/>
This revision contains the following additions/changes:
1. consistent usage of @Overrides
2. comments on unsynchronized methods depending on other synchronized
methods
3. overall more code coverage of insert, append, indexOf, lastIndexOf,
etc. for both StringBuffer and StringBuilder
4. testing of StringBuffer to ensure that all public un-synchronized
methods are actually synchronized along their call chain. the
StringBuffer/TestSynchronization class uses reflection to decide which
methods to test and could be (with a bit of work on method parameter
setup), be used to test any class for these synchronization
properties. The motivation for this test is to provide some degree of
assurance that modifications to StringBuffer, particularly addition of
new methods, do not break the synchronization contract. The code also
includes a self-test to guard against breaking the test program itself.
I'm skimmed over this and StringBuffer looks a lot better, thanks for
reverting back and adding the comments.
I don't have time to review the tests just now but it looks like you've
added a test of useful tests. I did look at
TestSynchronization.testSynchronized and the approach looks right. The
timeout is short so there will periodically be false positives but
that's okay as increasing the timeout will slow down the test (the main
thing I was looking for was the potential for false negatives).
-Alan
