axis-java-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From keith chapman <keithgchap...@gmail.com>
Subject Re: Axis2 API design issue, Should we fix it?
Date Mon, 11 May 2009 07:57:38 GMT
Shall we go ahead with this change then?

Thanks,
Keith.

On Wed, May 6, 2009 at 7:11 PM, Amila Suriarachchi <
amilasuriarachchi@gmail.com> wrote:

>
>
> 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/
>



-- 
Keith Chapman
Senior Software Engineer
WSO2 Inc.
Oxygenating the Web Service Platform.
http://wso2.org/

blog: http://www.keith-chapman.org

Mime
View raw message