Re: svn commit: r1482441 - /ofbiz/trunk/framework/entity/src/org/ofbiz/entity/condition/EntityFieldMap.java

[Adrian Crum-3]

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, [hidden email] 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;
>       }
>  
>
>

[Adam Heath-2]

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, [hidden email] 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;
>>       }
>>  
>>
>