You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@commons.apache.org by Robert Ribnitz <ro...@softwired.ch> on 2006/07/29 13:00:18 UTC

Minor improvements to UnmodifiableMap: local vars made final, only one return statement in the mapIterator.

Hello,

I had a look at UnmodifiableMap.

-in the mapIterator() the return statement can be taken out; we also 
need to define the it outside the if block then. Also made final.
-entrySet()/keySet()/values(): the local variable can be made final

I have attached an unidiff (diff -u..) against a 3.2 release build.

hope this helps

Robert Ribnitz


Re: [collections] Minor improvements to UnmodifiableMap: local vars made final, only one return statement in the mapIterator.

Posted by Stephen Colebourne <sc...@btopenworld.com>.
What do you see as the benefits of this? The code works fine currently, 
and this doesn't change that one way or the other.

Stephen

Robert Ribnitz wrote:
> Hello,
> 
> I had a look at UnmodifiableMap.
> 
> -in the mapIterator() the return statement can be taken out; we also 
> need to define the it outside the if block then. Also made final.
> -entrySet()/keySet()/values(): the local variable can be made final
> 
> I have attached an unidiff (diff -u..) against a 3.2 release build.
> 
> hope this helps
> 
> Robert Ribnitz
> 
> 
> ------------------------------------------------------------------------
> 
> --- ./commons-collections-3.2-src-orig/src/java/org/apache/commons/collections/map/UnmodifiableMap.java	2006-05-14 22:39:42.000000000 +0200
> +++ ./commons-collections-3.2-src/src/java/org/apache/commons/collections/map/UnmodifiableMap.java	2006-07-29 12:49:08.354541720 +0200
> @@ -116,27 +116,27 @@
>      }
>  
>      public MapIterator mapIterator() {
> +    	final MapIterator it;
>          if (map instanceof IterableMap) {
> -            MapIterator it = ((IterableMap) map).mapIterator();
> -            return UnmodifiableMapIterator.decorate(it);
> +             it = ((IterableMap) map).mapIterator();
>          } else {
> -            MapIterator it = new EntrySetMapIterator(map);
> -            return UnmodifiableMapIterator.decorate(it);
> +            it = new EntrySetMapIterator(map);
>          }
> +        return UnmodifiableMapIterator.decorate(it);
>      }
>  
>      public Set entrySet() {
> -        Set set = super.entrySet();
> +        final Set set = super.entrySet();
>          return UnmodifiableEntrySet.decorate(set);
>      }
>  
>      public Set keySet() {
> -        Set set = super.keySet();
> +        final Set set = super.keySet();
>          return UnmodifiableSet.decorate(set);
>      }
>  
>      public Collection values() {
> -        Collection coll = super.values();
> +        final Collection coll = super.values();
>          return UnmodifiableCollection.decorate(coll);
>      }
>  
> 
> 
> 
> ------------------------------------------------------------------------
> 
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: commons-dev-unsubscribe@jakarta.apache.org
> For additional commands, e-mail: commons-dev-help@jakarta.apache.org

---------------------------------------------------------------------
To unsubscribe, e-mail: commons-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: commons-dev-help@jakarta.apache.org