svn commit: r1836710 - in /ofbiz/ofbiz-framework/trunk/framework/base/src/main/java/org/apache/ofbiz/base/util/collections: MapContext.java MapStack.java

[Taher Alkhateeb]

Author: taher
Date: Thu Jul 26 10:02:06 2018
New Revision: 1836710

URL: http://svn.apache.org/viewvc?rev=1836710&view=rev
Log:
Improved: refactor the MapContext to use a Deque instead of List interface
(OFBIZ-10485)

The MapContext and MapStack classes are abstracting away the underlying
LinkedList implementation behind a List. The problem is that the push and pop
methods are implemented awkwardly by using index zero or using size() to find
the ends of the collection.

Thus this improvement maintains the same code signature but replaces the List
interface with the Deque to utilize the getFirst(), addFirst() and addLast()
functions making the code cleaner and more declaractive.

Thanks: Mathieu Lirzin for the patch and explanation of your work.

Modified:
    ofbiz/ofbiz-framework/trunk/framework/base/src/main/java/org/apache/ofbiz/base/util/collections/MapContext.java
    ofbiz/ofbiz-framework/trunk/framework/base/src/main/java/org/apache/ofbiz/base/util/collections/MapStack.java

Modified: ofbiz/ofbiz-framework/trunk/framework/base/src/main/java/org/apache/ofbiz/base/util/collections/MapContext.java
URL: http://svn.apache.org/viewvc/ofbiz/ofbiz-framework/trunk/framework/base/src/main/java/org/apache/ofbiz/base/util/collections/MapContext.java?rev=1836710&r1=1836709&r2=1836710&view=diff
==============================================================================
--- ofbiz/ofbiz-framework/trunk/framework/base/src/main/java/org/apache/ofbiz/base/util/collections/MapContext.java (original)
+++ ofbiz/ofbiz-framework/trunk/framework/base/src/main/java/org/apache/ofbiz/base/util/collections/MapContext.java Thu Jul 26 10:02:06 2018
@@ -22,6 +22,7 @@ import java.util.AbstractSet;
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.Collections;
+import java.util.Deque;
 import java.util.HashMap;
 import java.util.HashSet;
 import java.util.Iterator;
@@ -50,12 +51,11 @@ public class MapContext<K, V> implements
         super();
     }
 
-    protected List<Map<K, V>> stackList = new LinkedList<>();
+    protected Deque<Map<K, V>> stackList = new LinkedList<>();
 
     /** Puts a new Map on the top of the stack */
     public void push() {
-        Map<K, V> newMap = new HashMap<>();
-        this.stackList.add(0,newMap);
+        stackList.addFirst(new HashMap<K, V>());
     }
 
     /** Puts an existing Map on the top of the stack (top meaning will override lower layers on the stack) */
@@ -63,7 +63,7 @@ public class MapContext<K, V> implements
         if (existingMap == null) {
             throw new IllegalArgumentException("Error: cannot push null existing Map onto a MapContext");
         }
-        this.stackList.add(0, existingMap);
+        stackList.addFirst(existingMap);
     }
 
     /** Puts an existing Map on the BOTTOM of the stack (bottom meaning will be overriden by lower layers on the stack, ie everything else already there) */
@@ -71,16 +71,13 @@ public class MapContext<K, V> implements
         if (existingMap == null) {
             throw new IllegalArgumentException("Error: cannot add null existing Map to bottom of a MapContext");
         }
-        this.stackList.add(existingMap);
+        stackList.addLast(existingMap);
     }
 
     /** Remove and returns the Map from the top of the stack; if there is only one Map on the stack it returns null and does not remove it */
     public Map<K, V> pop() {
         // always leave at least one Map in the List, ie never pop off the last Map
-        if (this.stackList.size() > 1) {
-            return stackList.remove(0);
-        }
-        return null;
+        return stackList.size() > 1 ? stackList.removeFirst() : null;
     }
 
     /* (non-Javadoc)
@@ -188,8 +185,7 @@ public class MapContext<K, V> implements
     @Override
     public V put(K key, V value) {
         // all write operations are local: only put in the Map on the top of the stack
-        Map<K, V> currentMap = this.stackList.get(0);
-        return currentMap.put(key, value);
+        return stackList.getFirst().put(key, value);
     }
 
     /* (non-Javadoc)
@@ -198,8 +194,7 @@ public class MapContext<K, V> implements
     @Override
     public V remove(Object key) {
         // all write operations are local: only remove from the Map on the top of the stack
-        Map<K, V> currentMap = this.stackList.get(0);
-        return currentMap.remove(key);
+        return stackList.getFirst().remove(key);
     }
 
     /* (non-Javadoc)
@@ -208,8 +203,7 @@ public class MapContext<K, V> implements
     @Override
     public void putAll(Map<? extends K, ? extends V> arg0) {
         // all write operations are local: only put in the Map on the top of the stack
-        Map<K, V> currentMap = this.stackList.get(0);
-        currentMap.putAll(arg0);
+        stackList.getFirst().putAll(arg0);
     }
 
     /* (non-Javadoc)
@@ -218,7 +212,7 @@ public class MapContext<K, V> implements
     @Override
     public void clear() {
         // all write operations are local: only clear the Map on the top of the stack
-        this.stackList.get(0).clear();
+        stackList.getFirst().clear();
     }
 
     /* (non-Javadoc)

Modified: ofbiz/ofbiz-framework/trunk/framework/base/src/main/java/org/apache/ofbiz/base/util/collections/MapStack.java
URL: http://svn.apache.org/viewvc/ofbiz/ofbiz-framework/trunk/framework/base/src/main/java/org/apache/ofbiz/base/util/collections/MapStack.java?rev=1836710&r1=1836709&r2=1836710&view=diff
==============================================================================
--- ofbiz/ofbiz-framework/trunk/framework/base/src/main/java/org/apache/ofbiz/base/util/collections/MapStack.java (original)
+++ ofbiz/ofbiz-framework/trunk/framework/base/src/main/java/org/apache/ofbiz/base/util/collections/MapStack.java Thu Jul 26 10:02:06 2018
@@ -43,7 +43,7 @@ public class MapStack<K> extends MapCont
         if (baseMap instanceof MapStack) {
             newValue.stackList.addAll(((MapStack<K>) baseMap).stackList);
         } else {
-            newValue.stackList.add(0, baseMap);
+            newValue.stackList.addFirst(baseMap);
         }
         return newValue;
     }