Author: mbrohl
Date: Tue Oct 10 12:28:01 2017 New Revision: 1811685 URL: http://svn.apache.org/viewvc?rev=1811685&view=rev Log: Improved: Fixing defects reported by FindBugs, package org.apache.ofbiz.base.util.collections. (OFBIZ-9590) Thanks Dennis Balkir for reporting and providing the patch. Modified: ofbiz/ofbiz-framework/trunk/framework/base/src/main/java/org/apache/ofbiz/base/util/collections/FlexibleServletAccessor.java ofbiz/ofbiz-framework/trunk/framework/base/src/main/java/org/apache/ofbiz/base/util/collections/LifoSet.java ofbiz/ofbiz-framework/trunk/framework/base/src/main/java/org/apache/ofbiz/base/util/collections/MapComparator.java 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/ResourceBundleMapWrapper.java Modified: ofbiz/ofbiz-framework/trunk/framework/base/src/main/java/org/apache/ofbiz/base/util/collections/FlexibleServletAccessor.java URL: http://svn.apache.org/viewvc/ofbiz/ofbiz-framework/trunk/framework/base/src/main/java/org/apache/ofbiz/base/util/collections/FlexibleServletAccessor.java?rev=1811685&r1=1811684&r2=1811685&view=diff ============================================================================== --- ofbiz/ofbiz-framework/trunk/framework/base/src/main/java/org/apache/ofbiz/base/util/collections/FlexibleServletAccessor.java (original) +++ ofbiz/ofbiz-framework/trunk/framework/base/src/main/java/org/apache/ofbiz/base/util/collections/FlexibleServletAccessor.java Tue Oct 10 12:28:01 2017 @@ -66,7 +66,7 @@ public class FlexibleServletAccessor<T> } else { empty = false; int openPos = name.indexOf("${"); - if (openPos != -1 && name.indexOf("}", openPos) != -1) { + if (openPos != -1 && name.indexOf('}', openPos) != -1) { fma = null; attributeName = null; needsExpand = true; @@ -95,7 +95,7 @@ public class FlexibleServletAccessor<T> * @return the object corresponding to this getter class */ public T get(ServletRequest request, Map<String, Object> expandContext) { - AttributeAccessor<T> aa = new AttributeAccessor<T>(name, expandContext, this.attributeName, this.fma, this.needsExpand); + AttributeAccessor<T> aa = new AttributeAccessor<>(name, expandContext, this.attributeName, this.fma, this.needsExpand); return aa.get(request); } @@ -105,7 +105,7 @@ public class FlexibleServletAccessor<T> * @return the found value */ public T get(HttpSession session, Map<String, Object> expandContext) { - AttributeAccessor<T> aa = new AttributeAccessor<T>(name, expandContext, this.attributeName, this.fma, this.needsExpand); + AttributeAccessor<T> aa = new AttributeAccessor<>(name, expandContext, this.attributeName, this.fma, this.needsExpand); return aa.get(session); } @@ -119,7 +119,7 @@ public class FlexibleServletAccessor<T> * @param expandContext */ public void put(ServletRequest request, T value, Map<String, Object> expandContext) { - AttributeAccessor<T> aa = new AttributeAccessor<T>(name, expandContext, this.attributeName, this.fma, this.needsExpand); + AttributeAccessor<T> aa = new AttributeAccessor<>(name, expandContext, this.attributeName, this.fma, this.needsExpand); aa.put(request, value); } @@ -133,7 +133,7 @@ public class FlexibleServletAccessor<T> * @param expandContext */ public void put(HttpSession session, T value, Map<String, Object> expandContext) { - AttributeAccessor<T> aa = new AttributeAccessor<T>(name, expandContext, this.attributeName, this.fma, this.needsExpand); + AttributeAccessor<T> aa = new AttributeAccessor<>(name, expandContext, this.attributeName, this.fma, this.needsExpand); aa.put(session, value); } @@ -143,7 +143,7 @@ public class FlexibleServletAccessor<T> * @return the removed value */ public T remove(ServletRequest request, Map<String, Object> expandContext) { - AttributeAccessor<T> aa = new AttributeAccessor<T>(name, expandContext, this.attributeName, this.fma, this.needsExpand); + AttributeAccessor<T> aa = new AttributeAccessor<>(name, expandContext, this.attributeName, this.fma, this.needsExpand); return aa.remove(request); } @@ -153,11 +153,11 @@ public class FlexibleServletAccessor<T> * @return the removed value */ public T remove(HttpSession session, Map<String, Object> expandContext) { - AttributeAccessor<T> aa = new AttributeAccessor<T>(name, expandContext, this.attributeName, this.fma, this.needsExpand); + AttributeAccessor<T> aa = new AttributeAccessor<>(name, expandContext, this.attributeName, this.fma, this.needsExpand); return aa.remove(session); } - /** The equals and hashCode methods are imnplemented just case this object is ever accidently used as a Map key * + /** The equals and hashCode methods are implemented just case this object is ever accidently used as a Map key * * @return the hashcode */ @Override @@ -165,7 +165,7 @@ public class FlexibleServletAccessor<T> return this.name.hashCode(); } - /** The equals and hashCode methods are imnplemented just case this object is ever accidently used as a Map key + /** The equals and hashCode methods are implemented just case this object is ever accidently used as a Map key * @param obj * @return whether this object is equal to the passed object */ @@ -177,13 +177,14 @@ public class FlexibleServletAccessor<T> return flexibleServletAccessor.name == null; } return this.name.equals(flexibleServletAccessor.name); - } else { - String str = (String) obj; - if (this.name == null) { - return str == null; - } - return this.name.equals(str); } + if (this.name == null) { + return obj == null; + } + if (!(obj instanceof String)) { + return false; + } + return this.name.equals(obj); } /** To be used for a string representation of the accessor, returns the original name. @@ -258,9 +259,8 @@ public class FlexibleServletAccessor<T> if (fma != null) { return fma.get(UtilGenerics.<String, Object>checkMap(theValue)); - } else { - return UtilGenerics.<T>cast(theValue); } + return UtilGenerics.<T>cast(theValue); } public T get(HttpSession session) { @@ -274,9 +274,8 @@ public class FlexibleServletAccessor<T> if (fma != null) { return fma.get(UtilGenerics.<String, Object>checkMap(theValue)); - } else { - return UtilGenerics.<T>cast(theValue); } + return UtilGenerics.<T>cast(theValue); } protected void putInList(List<T> lst, T value) { @@ -336,19 +335,16 @@ public class FlexibleServletAccessor<T> if (isListReference) { List<Object> lst = UtilGenerics.checkList(theObj); return fma.remove(UtilGenerics.checkMap(lst.get(listIndex), String.class, Object.class)); - } else { - return fma.remove(UtilGenerics.checkMap(theObj, String.class, Object.class)); - } - } else { - if (isListReference) { - List<Object> lst = UtilGenerics.checkList(request.getAttribute(attributeName)); - return UtilGenerics.<T>cast(lst.remove(listIndex)); - } else { - Object theValue = request.getAttribute(attributeName); - request.removeAttribute(attributeName); - return UtilGenerics.<T>cast(theValue); } + return fma.remove(UtilGenerics.checkMap(theObj, String.class, Object.class)); + } + if (isListReference) { + List<Object> lst = UtilGenerics.checkList(request.getAttribute(attributeName)); + return UtilGenerics.<T>cast(lst.remove(listIndex)); } + Object theValue = request.getAttribute(attributeName); + request.removeAttribute(attributeName); + return UtilGenerics.<T>cast(theValue); } public T remove(HttpSession session) { @@ -357,19 +353,16 @@ public class FlexibleServletAccessor<T> if (isListReference) { List<Object> lst = UtilGenerics.checkList(theObj); return fma.remove(UtilGenerics.checkMap(lst.get(listIndex), String.class, Object.class)); - } else { - return fma.remove(UtilGenerics.checkMap(theObj, String.class, Object.class)); - } - } else { - if (isListReference) { - List<Object> lst = UtilGenerics.checkList(session.getAttribute(attributeName)); - return UtilGenerics.<T>cast(lst.remove(listIndex)); - } else { - Object theValue = session.getAttribute(attributeName); - session.removeAttribute(attributeName); - return UtilGenerics.<T>cast(theValue); } + return fma.remove(UtilGenerics.checkMap(theObj, String.class, Object.class)); + } + if (isListReference) { + List<Object> lst = UtilGenerics.checkList(session.getAttribute(attributeName)); + return UtilGenerics.<T>cast(lst.remove(listIndex)); } + Object theValue = session.getAttribute(attributeName); + session.removeAttribute(attributeName); + return UtilGenerics.<T>cast(theValue); } } } Modified: ofbiz/ofbiz-framework/trunk/framework/base/src/main/java/org/apache/ofbiz/base/util/collections/LifoSet.java URL: http://svn.apache.org/viewvc/ofbiz/ofbiz-framework/trunk/framework/base/src/main/java/org/apache/ofbiz/base/util/collections/LifoSet.java?rev=1811685&r1=1811684&r2=1811685&view=diff ============================================================================== --- ofbiz/ofbiz-framework/trunk/framework/base/src/main/java/org/apache/ofbiz/base/util/collections/LifoSet.java (original) +++ ofbiz/ofbiz-framework/trunk/framework/base/src/main/java/org/apache/ofbiz/base/util/collections/LifoSet.java Tue Oct 10 12:28:01 2017 @@ -1,4 +1,5 @@ /******************************************************************************* + * Licensed to the Apache Software Foundation (ASF) under one * or more contributor license agreements. See the NOTICE file * distributed with this work for additional information @@ -32,7 +33,7 @@ import java.util.LinkedList; public class LifoSet<V> extends AbstractSet<V> implements Serializable { // This set's back LinkedList - private LinkedList<V> backedList = new LinkedList<V>(); + private LinkedList<V> backedList = new LinkedList<>(); private int maxCapacity = 10; /** Modified: ofbiz/ofbiz-framework/trunk/framework/base/src/main/java/org/apache/ofbiz/base/util/collections/MapComparator.java URL: http://svn.apache.org/viewvc/ofbiz/ofbiz-framework/trunk/framework/base/src/main/java/org/apache/ofbiz/base/util/collections/MapComparator.java?rev=1811685&r1=1811684&r2=1811685&view=diff ============================================================================== --- ofbiz/ofbiz-framework/trunk/framework/base/src/main/java/org/apache/ofbiz/base/util/collections/MapComparator.java (original) +++ ofbiz/ofbiz-framework/trunk/framework/base/src/main/java/org/apache/ofbiz/base/util/collections/MapComparator.java Tue Oct 10 12:28:01 2017 @@ -48,8 +48,15 @@ public class MapComparator implements Co */ @Override public boolean equals(Object obj) { + if (obj==null) { + return false; + } return obj.equals(this); } + + public int hashCode() { + return super.hashCode(); + } /** * @see java.util.Comparator#compare(java.lang.Object, java.lang.Object) @@ -117,9 +124,8 @@ public class MapComparator implements Co if (compareResult != 0) { if (ascending) { return compareResult; - } else { - return -compareResult; } + return -compareResult; } } 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=1811685&r1=1811684&r2=1811685&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 Tue Oct 10 12:28:01 2017 @@ -43,7 +43,7 @@ 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<K, V>(); + return new MapContext<>(); } public static <K, V> MapContext<K, V> createMapContext() { @@ -75,15 +75,15 @@ public class MapContext<K, V> implements super(); } - protected List<Map<K, V>> stackList = new LinkedList<Map<K, V>>(); + protected List<Map<K, V>> stackList = new LinkedList<>(); public void reset() { - stackList = new LinkedList<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<K, V>(); + Map<K, V> newMap = new HashMap<>(); this.stackList.add(0,newMap); } @@ -108,9 +108,8 @@ public class MapContext<K, V> implements // 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); - } else { - return null; } + return null; } /** @@ -177,7 +176,7 @@ public class MapContext<K, V> implements */ public boolean containsValue(Object value) { // walk the stackList and the entries for each Map and if nothing is in for the current key, consider it an option, otherwise ignore - Set<K> resultKeySet = new HashSet<K>(); + Set<K> resultKeySet = new HashSet<>(); for (Map<K, V> curMap: this.stackList) { for (Map.Entry<K, V> curEntry: curMap.entrySet()) { if (!resultKeySet.contains(curEntry.getKey())) { @@ -222,9 +221,8 @@ public class MapContext<K, V> implements if (curMap instanceof LocalizedMap<?>) { LocalizedMap<V> lmap = UtilGenerics.cast(curMap); return lmap.get(name, locale); - } else { - return curMap.get(name); } + return curMap.get(name); } } return null; @@ -270,7 +268,7 @@ public class MapContext<K, V> implements */ public Set<K> keySet() { // walk the stackList and aggregate all keys - Set<K> resultSet = new HashSet<K>(); + Set<K> resultSet = new HashSet<>(); for (Map<K, V> curMap: this.stackList) { resultSet.addAll(curMap.keySet()); } @@ -282,8 +280,8 @@ public class MapContext<K, V> implements */ public Collection<V> values() { // 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<K>(); - List<V> resultValues = new LinkedList<V>(); + Set<K> resultKeySet = new HashSet<>(); + List<V> resultValues = new LinkedList<>(); for (Map<K, V> curMap: this.stackList) { for (Map.Entry<K, V> curEntry: curMap.entrySet()) { if (!resultKeySet.contains(curEntry.getKey())) { @@ -300,8 +298,8 @@ 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<K>(); - Set<Map.Entry<K, V>> resultEntrySet = new ListSet<Map.Entry<K, V>>(); + Set<K> resultKeySet = new HashSet<>(); + Set<Map.Entry<K, V>> resultEntrySet = new ListSet<>(); for (Map<K, V> curMap: this.stackList) { for (Map.Entry<K, V> curEntry: curMap.entrySet()) { if (!resultKeySet.contains(curEntry.getKey())) { @@ -338,12 +336,12 @@ public class MapContext<K, V> implements return fullMapString.toString(); } - private static final class ListSet<E> extends AbstractSet<E> implements Set<E> { + private static final class ListSet<E> extends AbstractSet<E> { - protected final List<E> listImpl; + private final List<E> listImpl; public ListSet() { - this.listImpl = new ArrayList<E>(); + this.listImpl = new ArrayList<>(); } public int size() { Modified: ofbiz/ofbiz-framework/trunk/framework/base/src/main/java/org/apache/ofbiz/base/util/collections/ResourceBundleMapWrapper.java URL: http://svn.apache.org/viewvc/ofbiz/ofbiz-framework/trunk/framework/base/src/main/java/org/apache/ofbiz/base/util/collections/ResourceBundleMapWrapper.java?rev=1811685&r1=1811684&r2=1811685&view=diff ============================================================================== --- ofbiz/ofbiz-framework/trunk/framework/base/src/main/java/org/apache/ofbiz/base/util/collections/ResourceBundleMapWrapper.java (original) +++ ofbiz/ofbiz-framework/trunk/framework/base/src/main/java/org/apache/ofbiz/base/util/collections/ResourceBundleMapWrapper.java Tue Oct 10 12:28:01 2017 @@ -174,13 +174,13 @@ public class ResourceBundleMapWrapper im // when the main Map doesn't have a certain value if (resourceBundle != null) { Set<String> set = resourceBundle.keySet(); - topLevelMap = new HashMap<String, Object>(set.size()); + topLevelMap = new HashMap<>(set.size()); for (String key : set) { Object value = resourceBundle.getObject(key); topLevelMap.put(key, value); } } else { - topLevelMap = new HashMap<String, Object>(1); + topLevelMap = new HashMap<>(1); } topLevelMap.put("_RESOURCE_BUNDLE_", resourceBundle); isMapInitialized = true; @@ -190,13 +190,12 @@ public class ResourceBundleMapWrapper im /* (non-Javadoc) * @see java.util.Map#size() */ - public int size() { + public int size() { if(isMapInitialized) { // this is an approximate size, won't include elements from parent bundles return topLevelMap.size() -1; - } else { - return resourceBundle.keySet().size(); } + return resourceBundle.keySet().size(); } /* (non-Javadoc) @@ -205,9 +204,8 @@ public class ResourceBundleMapWrapper im public boolean isEmpty() { if (isMapInitialized) { return topLevelMap.isEmpty(); - } else { - return resourceBundle.keySet().size() == 0; } + return resourceBundle.keySet().size() == 0; } /* (non-Javadoc) @@ -220,9 +218,10 @@ public class ResourceBundleMapWrapper im } } else { try { - if (this.resourceBundle.getObject((String) arg0) != null) { - return true; - } + //the following will just be executed to check if arg0 is null, + //if so the thrown exception will be caught, if not true will be returned + this.resourceBundle.getObject((String) arg0); + return true; } catch (NullPointerException e) { // happens when arg0 is null } catch (MissingResourceException e) { |
Free forum by Nabble | Edit this page |