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