You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@ofbiz.apache.org by Adrian Crum <ad...@sandglass-software.com> on 2013/05/14 19:06:40 UTC
Re: svn commit: r1482441 - /ofbiz/trunk/framework/entity/src/org/ofbiz/entity/condition/EntityFieldMap.java
This might cause problems. The empty collections are immutable, and
client code might try to add elements to them.
-Adrian
On 5/14/2013 5:38 PM, doogie@apache.org wrote:
> Author: doogie
> Date: Tue May 14 16:38:30 2013
> New Revision: 1482441
>
> URL: http://svn.apache.org/r1482441
> Log:
> FEATURE: Remove use of empty ArrayList/HashMap, instead using
> Collections emptyList/emptyMap.
>
> Modified:
> ofbiz/trunk/framework/entity/src/org/ofbiz/entity/condition/EntityFieldMap.java
>
> Modified: ofbiz/trunk/framework/entity/src/org/ofbiz/entity/condition/EntityFieldMap.java
> URL: http://svn.apache.org/viewvc/ofbiz/trunk/framework/entity/src/org/ofbiz/entity/condition/EntityFieldMap.java?rev=1482441&r1=1482440&r2=1482441&view=diff
> ==============================================================================
> --- ofbiz/trunk/framework/entity/src/org/ofbiz/entity/condition/EntityFieldMap.java (original)
> +++ ofbiz/trunk/framework/entity/src/org/ofbiz/entity/condition/EntityFieldMap.java Tue May 14 16:38:30 2013
> @@ -23,7 +23,6 @@ import java.util.Collections;
> import java.util.Iterator;
> import java.util.List;
> import java.util.Map;
> -import java.util.HashMap;
>
> import org.ofbiz.base.util.UtilGenerics;
> import org.ofbiz.entity.util.EntityUtil;
> @@ -43,7 +42,7 @@ public class EntityFieldMap extends Enti
>
> public static <V> List<EntityExpr> makeConditionList(Map<String, V> fieldMap, EntityComparisonOperator<?,V> op) {
> if (fieldMap == null) {
> - return new ArrayList<EntityExpr>();
> + return Collections.emptyList();
> }
> List<EntityExpr> list = new ArrayList<EntityExpr>(fieldMap.size());
> for (Map.Entry<String, ? extends Object> entry: fieldMap.entrySet()) {
> @@ -54,19 +53,14 @@ public class EntityFieldMap extends Enti
>
> public <V> void init(EntityComparisonOperator<?,?> compOp, EntityJoinOperator joinOp, V... keysValues) {
> super.init(makeConditionList(EntityUtil.makeFields(keysValues), UtilGenerics.<EntityComparisonOperator<String,V>>cast(compOp)), joinOp);
> - this.fieldMap = EntityUtil.makeFields(keysValues);
> - if (this.fieldMap == null) {
> - this.fieldMap = new HashMap<String, Object>();
> - }
> + Map<String, ? extends Object> fieldMap = EntityUtil.makeFields(keysValues);
> + this.fieldMap = fieldMap == null ? Collections.<String, Object>emptyMap() : fieldMap;
> this.operator = joinOp;
> }
>
> public <V> void init(Map<String, V> fieldMap, EntityComparisonOperator<?,?> compOp, EntityJoinOperator joinOp) {
> super.init(makeConditionList(fieldMap, UtilGenerics.<EntityComparisonOperator<String,V>>cast(compOp)), joinOp);
> - this.fieldMap = fieldMap;
> - if (this.fieldMap == null) {
> - this.fieldMap = new HashMap<String, Object>();
> - }
> + this.fieldMap = fieldMap == null ? Collections.<String, Object>emptyMap() : fieldMap;
> this.operator = joinOp;
> }
>
>
>
Re: svn commit: r1482441 - /ofbiz/trunk/framework/entity/src/org/ofbiz/entity/condition/EntityFieldMap.java
Posted by Adam Heath <do...@brainfood.com>.
Nope. fieldMap is a protected field, there is no way to add or change
things in it. Even the accessor methods pass the map thru
unmodifiableMap().
On 05/14/2013 12:06 PM, Adrian Crum wrote:
> This might cause problems. The empty collections are immutable, and
> client code might try to add elements to them.
>
> -Adrian
>
> On 5/14/2013 5:38 PM, doogie@apache.org wrote:
>> Author: doogie
>> Date: Tue May 14 16:38:30 2013
>> New Revision: 1482441
>>
>> URL: http://svn.apache.org/r1482441
>> Log:
>> FEATURE: Remove use of empty ArrayList/HashMap, instead using
>> Collections emptyList/emptyMap.
>>
>> Modified:
>>
>> ofbiz/trunk/framework/entity/src/org/ofbiz/entity/condition/EntityFieldMap.java
>>
>>
>> Modified:
>> ofbiz/trunk/framework/entity/src/org/ofbiz/entity/condition/EntityFieldMap.java
>>
>> URL:
>> http://svn.apache.org/viewvc/ofbiz/trunk/framework/entity/src/org/ofbiz/entity/condition/EntityFieldMap.java?rev=1482441&r1=1482440&r2=1482441&view=diff
>>
>> ==============================================================================
>>
>> ---
>> ofbiz/trunk/framework/entity/src/org/ofbiz/entity/condition/EntityFieldMap.java
>> (original)
>> +++
>> ofbiz/trunk/framework/entity/src/org/ofbiz/entity/condition/EntityFieldMap.java
>> Tue May 14 16:38:30 2013
>> @@ -23,7 +23,6 @@ import java.util.Collections;
>> import java.util.Iterator;
>> import java.util.List;
>> import java.util.Map;
>> -import java.util.HashMap;
>> import org.ofbiz.base.util.UtilGenerics;
>> import org.ofbiz.entity.util.EntityUtil;
>> @@ -43,7 +42,7 @@ public class EntityFieldMap extends Enti
>> public static <V> List<EntityExpr>
>> makeConditionList(Map<String, V> fieldMap,
>> EntityComparisonOperator<?,V> op) {
>> if (fieldMap == null) {
>> - return new ArrayList<EntityExpr>();
>> + return Collections.emptyList();
>> }
>> List<EntityExpr> list = new
>> ArrayList<EntityExpr>(fieldMap.size());
>> for (Map.Entry<String, ? extends Object> entry:
>> fieldMap.entrySet()) {
>> @@ -54,19 +53,14 @@ public class EntityFieldMap extends Enti
>> public <V> void init(EntityComparisonOperator<?,?> compOp,
>> EntityJoinOperator joinOp, V... keysValues) {
>>
>> super.init(makeConditionList(EntityUtil.makeFields(keysValues),
>> UtilGenerics.<EntityComparisonOperator<String,V>>cast(compOp)),
>> joinOp);
>> - this.fieldMap = EntityUtil.makeFields(keysValues);
>> - if (this.fieldMap == null) {
>> - this.fieldMap = new HashMap<String, Object>();
>> - }
>> + Map<String, ? extends Object> fieldMap =
>> EntityUtil.makeFields(keysValues);
>> + this.fieldMap = fieldMap == null ? Collections.<String,
>> Object>emptyMap() : fieldMap;
>> this.operator = joinOp;
>> }
>> public <V> void init(Map<String, V> fieldMap,
>> EntityComparisonOperator<?,?> compOp, EntityJoinOperator joinOp) {
>> super.init(makeConditionList(fieldMap,
>> UtilGenerics.<EntityComparisonOperator<String,V>>cast(compOp)),
>> joinOp);
>> - this.fieldMap = fieldMap;
>> - if (this.fieldMap == null) {
>> - this.fieldMap = new HashMap<String, Object>();
>> - }
>> + this.fieldMap = fieldMap == null ? Collections.<String,
>> Object>emptyMap() : fieldMap;
>> this.operator = joinOp;
>> }
>>
>>
>