On Mon, May 11, 2009 at 1:45 PM, Andreas Veithen
<andreas.veit...@gmail.com>wrote:
> +1, but we need to agree on the target release for this change.
I think this should not go to 1.5 and only apply to axis2 trunk (i.e 1.6).
thanks,
Amila.
>
>
> Andreas
>
> On Mon, May 11, 2009 at 09:57, keith chapman <keithgchap...@gmail.com>
> wrote:
> > Shall we go ahead with this change then?
> >
> > Thanks,
> > Keith.
> >
> > On Wed, May 6, 2009 at 7:11 PM, Amila Suriarachchi
> > <amilasuriarach...@gmail.com> wrote:
> >>
> >>
> >> On Wed, May 6, 2009 at 4:33 PM, Glen Daniels <g...@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/
> >
> >
> >
> > --
> > Keith Chapman
> > Senior Software Engineer
> > WSO2 Inc.
> > Oxygenating the Web Service Platform.
> > http://wso2.org/
> >
> > blog: http://www.keith-chapman.org
> >
>
--
Amila Suriarachchi
WSO2 Inc.
blog: http://amilachinthaka.blogspot.com/
