I don't think we can put it into 1.5. Lets make the change in the trunk so
that it will be available in the next release.
Thanks,
Keith.
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.
>
> 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
> >
>
--
Keith Chapman
Senior Software Engineer
WSO2 Inc.
Oxygenating the Web Service Platform.
http://wso2.org/
blog: http://www.keith-chapman.org
