axis-java-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Amila Suriarachchi <amilasuriarach...@gmail.com>
Subject Re: Axis2 API design issue, Should we fix it?
Date Wed, 06 May 2009 13:41:37 GMT
On Wed, May 6, 2009 at 4:33 PM, Glen Daniels <glen@thoughtcraft.com> wrote:

> Amila Suriarachchi wrote:
> >     Hm - it seems like you didn't actually get my point.  We CAN'T
> >     return the
> >     actual allServices map because that would be breaking the abstraction
> >     boundary provided by the class.
> >
> > As I remember this change was done to avoid concurrent modifications to
> > service map[1].
>
> Right - before this change we were doing something actively bad/wrong, i.e.
> returning the internal map.  After the change we were returning a cloned
> copy
> of the map (though not using clone() for some reason), which is good in
> that
> it prevented people from misusing it.
>
> > At that time services map was a HashMap and could not fix the issue by
> > changing it to a ConcurrentHashMap since API method returns a HashMap.
> >
> > Currently anyone can get a copy of internal map (I think we can not use
> > clone() method since internal implementation is ConcurrentHashMap and we
> > need to return a HashMap). And if they want to add or remove service
> > they have to use addService and removeService respectively.
> >
> > Keith, if you really need the internal map IMHO best way is to add a new
> > API method.
>
> Ah, no.  The "best way" is NOT TO OFFER ACCESS TO THE INTERNAL MAP.
>
> Keith's suggestion is to change the API so that it returns a Map - this
> would
> be much more correct since it increases the level of abstraction for the
> method, and would therefore let us, the implementors, freely decide how
> this
> should work internally.  Right now we have problems because someone made
> this
> method overly specific, and this is what we should fix.  (I was incorrect
> earlier when I said this wouldn't break people, btw - I was thinking about
> stuff like getServices().get("MyService"), but of course "HashMap foo =
> getServices()" would fail.  I still think we should fix it.)
>
> My comment is that we should not only return a Map, we should change the
> implementation to make sure the Map is immutable, and make sure the JavaDoc
> explains what is going on.

+1. Here the real problem is this API contains a hashMap instead of Map.

What I got from the Keiths' first mail is that he need the API change to
return the internal map without copying. I suggested to add a new method if
he really need it so that only he use that method. I agree with you that
this is not a good change.

thanks,
Amila.

>
>
> Make sense?
>
> --Glen
>



-- 
Amila Suriarachchi
WSO2 Inc.
blog: http://amilachinthaka.blogspot.com/

Mime
View raw message