On 10/11/2014 8:17 AM, Remi Forax wrote:
Hi Mandy,
On 10/11/2014 02:31 AM, Mandy Chung wrote:
[...]
webrev: http://cr.openjdk.java.net/~redestad/8060130/webrev.02/
ClassLoader.java
line 272: can you change the declared type as Map.
I think it's a bad idea, the class doesn't work if you replace the
ConcurrentHashMap by any Map
so declaring the field 'packages' as a Map seems wrong to me.
At least, it should be a ConcurrentMap, but even with a ConcurrentMap,
I think that the guarantee
that in definePackage the call to putIfAbstent is amortized O(1) and
not something like O(ln n) is important.
Note that using interfaces instead of implementations is important in
the signature of public or protected method, for a local variable or a
private field, which are hidden, there is no real reason to use an
interface. I understand that pedagogically having only one rule is
appealing but from my own experience, this rule hide one of the corner
stone of the OOP, encapsulation, so IMO the rule 'use an interface
everywhere you can' does more harm than good because it offers a
discorded version of the OO world.
Remi - thanks for catching this. After I sent the comment, I was
pondering my suggestion (this specific one) was a bad idea. Declaring it
as Map loses the significance that the implementation depends on the
packages map being concurrently accessed. Agree.
Mandy
