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