Am 19.03.2012 05:53, schrieb Sean Chou:
Hi Ulf,
I hope following comments can help reduce your concern.
I don't think this footprint is a problem until it is proved to be one. If it is, a better javac
can be used to save this footprint, and it would save much more. The actual problem might be
reached by http://openjdk.java.net/jeps/149 .
As I have no setup here now, I cna't proove it, sorry, but: Many a mickle makes
a muckle.
With the if statement, it reads "put a null after the elements if there are
more space",
while with your code, it reads "copy all the elements from r or copy
all elements and 1
more from r if there are more space" and we have to think "what's the
next element in r ?
". In fact, we need look back to find how r is defined "T[] r = a.length
>= size ? a :
(T[])java.lang.reflect.Array.newInstance(a.getClass().getComponentType(),
size);" and go
through the code once more to realize there is a null at that position.
Yes, a better comment would be necessary.
A comment just help understand the code, does not remove this thinking.
Well, but only if one doesn't believe it ;-)
However, 7153238 is a RFE and this is a bug. I would like the bug to be fixed in a way easy to
understand if possible. And you can put all the enhancement in the RFE which in fact would do much
more then this piece of byte code footprint saving. It is better to do one thing in one bug/RFE.
Agreed!
But you could reuse it (like Object[] res) with:
Map<String,Object> map = new TConcurrentHashMap<>(); // better: TCHM1
...
map = new TConcurrentHashMap2<>();
I agree, a little nit, but I had the other "missing" test cases in mind,
where the numbering
then could become confusing.
The comment"// Check less elements" and " // Check equal elements" clearly describe the two blocks
of code are testing different scenarios.
And a new definition would emphasize it is a different variable, it is used to test different
scenarios.
With same justification you could have: a1, a2, res1, res2 instead reusing a,
res.
I would not like to add "// inherits from AbstractCollection.toArray()" ,
it is obvious and
listed in java doc.
In ConcurrentHashMap.values:
Overrides: values in class AbstractMap<K,V>
In AbstractMap.values:
This implementation returns a collection that subclasses
AbstractCollection.
But there is no guarantee, that the returned collection overrides or just
delegates to
AbstractCollection.toArray().
In worst case, t.j.u.AC.ToArray doesn't test j.u.
AbstractCollection.toArray() at all.
Do you mean ConcurrentHashMap returns a collection does not inherit AbstractCollection ? Don't
worry about that, ConcurrentHashMap is a class of java collection framework, it won't let that
happen. AbstractCollection is born to be inherited, especially by collection classes in java
collection framework.
Yes, it is born to be inherited, and one day someone could override AbstractCollection.toArray() in
the implementation of the subclassed collection returned by AbstractMap.values() without violating
the given spec. In this case, your test would test the subclassed collection's toArray() method, but
not AbstractCollection.toArray().
-Ulf
