Hi Keith!

keith chapman wrote:
> Due to a design issue in the Axis2 API we have implemented the
> getServices() method as follows,
> 
> public HashMap<String, AxisService> getServices() {
>         HashMap<String, AxisService> hashMap = new HashMap<String,
> AxisService>(this.allServices.size());
>         String key;
>         for (Iterator<String> iter =
> this.allServices.keySet().iterator(); iter.hasNext();){
>             key = iter.next();
>             hashMap.put(key, this.allServices.get(key));
>         }
>         return hashMap;
>     }
> 
> In my view this is highly inefficient, especially if you have plenty of
> services in the system. Wouldn't it be better to fix the API and return
> a Map instead of a HashMap? If we did that we could simple return
> allServices instead of returning a copy of it.

You don't want to return an actual reference to a mutable object that backs a
significant data model - otherwise people could just get that Map and
(mistakenly or maliciously) randomly add and delete services.

However, your point about the HashMap in the API is entirely right - we
should make it a Map (which shouldn't break anyone already using it in it's
more specific form).  Also, I'm not sure why we're not just returning
allServices.clone()... NIH? :)

> If we are making this change I propose that we fix this for modules,
> transports as well.

Sure.

--Glen

Reply via email to