svn commit: r1837409 - in /ofbiz/ofbiz-framework/trunk/framework: base/src/main/java/org/apache/ofbiz/base/util/collections/MapContext.java webapp/src/main/java/org/apache/ofbiz/webapp/control/ConfigXMLReader.java

[Taher Alkhateeb]

Author: taher
Date: Sat Aug  4 08:39:16 2018
New Revision: 1837409

URL: http://svn.apache.org/viewvc?rev=1837409&view=rev
Log:
Improved: refactored the MapContext object with multiple improvements
(OFBIZ-10485)

- Removed the constructor and factory method "getMapContext" as it was
  redundant and does not add any value
- Replaced manual for-loop for collection walking in multiple methods with
  simple stream calls applied declaratively with less noise in the code.
- Fully removed the ListSet custom data structure with as that data structure
  did not serve any meaningful purpose and it is always better to rely on
  the java builtin data structures where possible. Construction was replaced
  with a HashSet instead.

Thanks: Mathieu Lirzin for your time, discussions and patch.

Modified:
    ofbiz/ofbiz-framework/trunk/framework/base/src/main/java/org/apache/ofbiz/base/util/collections/MapContext.java
    ofbiz/ofbiz-framework/trunk/framework/webapp/src/main/java/org/apache/ofbiz/webapp/control/ConfigXMLReader.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=1837409&r1=1837408&r2=1837409&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 Sat Aug  4 08:39:16 2018
@@ -18,19 +18,17 @@
  *******************************************************************************/
 package org.apache.ofbiz.base.util.collections;
 
-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;
 import java.util.LinkedList;
 import java.util.List;
 import java.util.Locale;
 import java.util.Map;
 import java.util.Set;
+import java.util.stream.Collectors;
 
 import org.apache.ofbiz.base.util.UtilGenerics;
 
@@ -43,14 +41,6 @@ public class MapContext<K, V> implements
 
     public static final String module = MapContext.class.getName();
 
-    public static final <K, V> MapContext<K, V> getMapContext() {
-        return new MapContext<>();
-    }
-
-    protected MapContext() {
-        super();
-    }
-
     protected Deque<Map<K, V>> stackList = new LinkedList<>();
 
     /** Puts a new Map on the top of the stack */
@@ -96,13 +86,7 @@ public class MapContext<K, V> implements
      */
     @Override
     public boolean isEmpty() {
-        // walk the stackList and if any is not empty, return false; otherwise return true
-        for (Map<K, V> curMap: this.stackList) {
-            if (!curMap.isEmpty()) {
-                return false;
-            }
-        }
-        return true;
+        return stackList.stream().allMatch(Map::isEmpty);
     }
 
     /* (non-Javadoc)
@@ -110,13 +94,7 @@ public class MapContext<K, V> implements
      */
     @Override
     public boolean containsKey(Object key) {
-        // walk the stackList and for the first place it is found return true; otherwise refurn false
-        for (Map<K, V> curMap: this.stackList) {
-            if (curMap.containsKey(key)) {
-                return true;
-            }
-        }
-        return false;
+        return stackList.stream().anyMatch(m -> m.containsKey(key));
     }
 
     /* (non-Javadoc)
@@ -220,12 +198,10 @@ public class MapContext<K, V> implements
      */
     @Override
     public Set<K> keySet() {
-        // walk the stackList and aggregate all keys
-        Set<K> resultSet = new HashSet<>();
-        for (Map<K, V> curMap: this.stackList) {
-            resultSet.addAll(curMap.keySet());
-        }
-        return Collections.unmodifiableSet(resultSet);
+        Set<K> keys = stackList.stream()
+                .flatMap(m -> m.keySet().stream())
+                .collect(Collectors.toSet());
+        return Collections.unmodifiableSet(keys);
     }
 
     /* (non-Javadoc)
@@ -254,7 +230,7 @@ public class MapContext<K, V> implements
     public Set<Map.Entry<K, V>> entrySet() {
         // walk the stackList and the entries for each Map and if nothing is in for the current key, put it in
         Set<K> resultKeySet = new HashSet<>();
-        Set<Map.Entry<K, V>> resultEntrySet = new ListSet<>();
+        Set<Map.Entry<K, V>> resultEntrySet = new HashSet<>();
         for (Map<K, V> curMap: this.stackList) {
             for (Map.Entry<K, V> curEntry: curMap.entrySet()) {
                 if (!resultKeySet.contains(curEntry.getKey())) {
@@ -290,49 +266,4 @@ public class MapContext<K, V> implements
         }
         return fullMapString.toString();
     }
-
-    private static final class ListSet<E> extends AbstractSet<E> {
-
-        private final List<E> listImpl;
-
-        public ListSet() {
-            this.listImpl = new ArrayList<>();
-        }
-
-        public int size() {
-            return this.listImpl.size();
-        }
-
-        public Iterator<E> iterator() {
-            return this.listImpl.iterator();
-        }
-
-        public boolean add(final E obj) {
-            boolean added = false;
-
-            if (!this.listImpl.contains(obj)) {
-                added = this.listImpl.add(obj);
-            }
-
-            return added;
-        }
-
-        public boolean isEmpty() {
-            return this.listImpl.isEmpty();
-        }
-
-        public boolean contains(final Object obj) {
-            return this.listImpl.contains(obj);
-        }
-
-        public boolean remove(final Object obj) {
-            return this.listImpl.remove(obj);
-        }
-
-        public void clear() {
-            this.listImpl.clear();
-        }
-
-    }
-
 }

Modified: ofbiz/ofbiz-framework/trunk/framework/webapp/src/main/java/org/apache/ofbiz/webapp/control/ConfigXMLReader.java
URL: http://svn.apache.org/viewvc/ofbiz/ofbiz-framework/trunk/framework/webapp/src/main/java/org/apache/ofbiz/webapp/control/ConfigXMLReader.java?rev=1837409&r1=1837408&r2=1837409&view=diff
==============================================================================
--- ofbiz/ofbiz-framework/trunk/framework/webapp/src/main/java/org/apache/ofbiz/webapp/control/ConfigXMLReader.java (original)
+++ ofbiz/ofbiz-framework/trunk/framework/webapp/src/main/java/org/apache/ofbiz/webapp/control/ConfigXMLReader.java Sat Aug  4 08:39:16 2018
@@ -214,7 +214,7 @@ public class ConfigXMLReader {
         }
 
         private <K, V> Map<K, V> pushIncludes(Function<ControllerConfig, Map<K, V>> f) throws WebAppConfigurationException {
-            MapContext<K, V> res = MapContext.getMapContext();
+            MapContext<K, V> res = new MapContext<>();
             for (URL include : includes) {
                 res.push(getControllerConfig(include).pushIncludes(f));
             }
@@ -277,7 +277,7 @@ public class ConfigXMLReader {
         }
 
         public Map<String, RequestMap> getRequestMapMap() throws WebAppConfigurationException {
-            MapContext<String, RequestMap> result = MapContext.getMapContext();
+            MapContext<String, RequestMap> result = new MapContext<>();
             for (URL includeLocation : includes) {
                 ControllerConfig controllerConfig = getControllerConfig(includeLocation);
                 result.push(controllerConfig.getRequestMapMap());