Still looks okay to me.
David
On 13/03/2012 4:58 PM, Sean Chou wrote:
Hi Ulf and David,
I modified the patch and added the testcase, it's now :
http://cr.openjdk.java.net/~zhouyx/7121314/webrev.02/ .
Ulf's compact version is used, it looks beautiful; however I
replaced the Math.min part with if statement because if statement is
more intuitive and I don't think there is any performance concern. But
it is not so compact now...
Also I added the equal size case and @author to testcase.
There is a little problem when I created the webrev, I don't know
how to change the "contributed-by" information for the testcase, so the
list is still Ulf's and my emails.
Please take a look again.
On Fri, Mar 9, 2012 at 8:45 PM, Ulf Zibis <[email protected]
<mailto:[email protected]>> wrote:
Am 09.03.2012 09:16, schrieb Sean Chou:
Hi all,
AbstractCollection.toArray(T[] ) might return a new array
even if the given array has enough room for the returned
elements when it is concurrently modified. This behavior
violates the spec documented in java.util.Collection .
This patch checks the size of returned array and copies the
elements to return to the given array if it is large enough.
The webrev is at :
http://cr.openjdk.java.net/~__zhouyx/7121314/webrev.00/
<http://cr.openjdk.java.net/~zhouyx/7121314/webrev.00/>
<http://cr.openjdk.java.net/%__7Ezhouyx/7121314/webrev.00/
<http://cr.openjdk.java.net/%7Ezhouyx/7121314/webrev.00/>>
More compact and marginally faster:
182 if (!it.hasNext()) { // fewer elements than expected
183 if (a == r) {
184 a[i] = null; // null-terminate
185 } else if (a.length < i) {
186 return Arrays.copyOf(r, i);
187 } else {
188 System.arraycopy(r, 0, a, 0, Math.min(++i,
a.length()); // ensure null-termination
189 }
190 return a;
191 }
There is a test case in the previous discussion. It is not
included in the webrev, because the testcase is heavily
implementation dependent. I will add it if it is requested.
I think, we should have a testcase for all 3 cases: fewer / equal /
less elements than expected.
Additionally I think, the correct null-termination should be tested.
Thread[] threads = new Thread[2];
threads[0] = new Thread(new ToArrayThread());
threads[1] = new Thread(new RemoveThread());
Why so complicated?
IMHO better:
Thread toArrayThread = new Thread(new ToArrayThread());
Thread removeThread = new Thread(new RemoveThread());
- Ulf
--
Best Regards,
Sean Chou
