Request for review : 7121314 : Behavior mismatch between AbstractCollection.toArray(T[] ) and its spec
Ulf Zibis
Ulf.Zibis at gmx.de
Fri Mar 23 21:09:26 UTC 2012
Hi Sean,
Am 23.03.2012 07:45, schrieb Sean Chou:
> Hi Ulf,
>
> I'm sorry I didn't quite get your testcase.
This is not a testcase, just a draft of an additionally more flexible test object to replace
TConcurrentHashMap/2.
> Will you please provide a jtreg style testcase with main method ?
Well, as I'm missing your agreement, that David's test implementation doesn't guarantee to test the
right toArray method of AbstractCollection as I explained before, I'm afraid that additional effort
would be for garbage.
Aside, as the instantiation of (several) ConcurrentHashMap subclassed test objects seems more
expensive, I believe, my simple TestCollection would increase the performance of the testcases.
> I was thinking you were worried by the comment "// inherits from AbstractCollection.toArray()"
> and "map2", if that was the case, I would like to modify.
This is additionally true, but not my main issue, as later to me it turned out, that "// inherits
from AbstractCollection.toArray()" is not enough to guarantee that the right toArray method is
actually tested, aside to look not obvious IMO.
-Ulf
P.S.: better rename to:
void setPseudoConcurrentSizeCourse(int... sizes) {...}
>
> On Fri, Mar 23, 2012 at 5:57 AM, Ulf Zibis <Ulf.Zibis at gmx.de <mailto:Ulf.Zibis at gmx.de>> wrote:
>
> Hi Sean,
>
> bad news ;-) ...
>
> Am 22.03.2012 08:28, schrieb Sean Chou:
>
> Hi Ulf,
>
> I'm glad you agreed my suggestion.
>
> To all:
> Can this patch be committed as it has been reviewed by David Holmes and Mike Duigou,
> and Ulf also says agreed ?
>
>
> I agree with your implementation of AbstractCollection, but NOT with the test.
> For correct testing I suggest to use:
>
> /**
> *
> * @author Ulf Zibis
> */
> public class TestCollection<E> extends AbstractCollection<E> {
>
> private E[] elements;
> private int[] sizes;
> private int nextSize;
>
> public TestCollection(E[] elements) {
> this.elements = elements;
> setConcurrentSizeCourse(null);
> }
>
> void setConcurrentSizeCourse(int... sizes) {
> this.sizes = sizes == null ? new int[]{elements.length} : sizes;
> nextSize = 0;
> }
>
> @Override
> public int size() {
> return sizes[nextSize == sizes.length-1 ? nextSize : nextSize++];
> }
>
> @Override
> public Iterator<E> iterator() {
> return new Iterator<>() {
>
> int pos = 0;
>
> public boolean hasNext() {
> return pos < sizes[nextSize];
> }
>
> public E next() {
> return elements[pos++];
> }
>
> public void remove() {
> throw new UnsupportedOperationException("Not supported yet.");
> }
> };
> }
> }
>
> -Ulf
>
>
>
>
> --
> Best Regards,
> Sean Chou
>
More information about the core-libs-dev
mailing list