You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@ofbiz.apache.org by Scott Gray <sc...@hotwaxmedia.com> on 2011/06/26 23:32:23 UTC
Re: svn commit: r1139876 - /ofbiz/trunk/framework/entity/src/org/ofbiz/entity/model/ModelViewEntity.java
Bit of back and forth regarding == vs. equals: http://svn.apache.org/viewvc?view=revision&revision=810438
Regards
Scott
On 27/06/2011, at 6:10 AM, doogie@apache.org wrote:
> Author: doogie
> Date: Sun Jun 26 18:10:10 2011
> New Revision: 1139876
>
> URL: http://svn.apache.org/viewvc?rev=1139876&view=rev
> Log:
> OPTIMIZE: ViewConditionExpr.createCondition: Use == for EntityOperator
> equality, instead of equals(); slight speedup, and the code looks a bit nicer.
>
> Modified:
> ofbiz/trunk/framework/entity/src/org/ofbiz/entity/model/ModelViewEntity.java
>
> Modified: ofbiz/trunk/framework/entity/src/org/ofbiz/entity/model/ModelViewEntity.java
> URL: http://svn.apache.org/viewvc/ofbiz/trunk/framework/entity/src/org/ofbiz/entity/model/ModelViewEntity.java?rev=1139876&r1=1139875&r2=1139876&view=diff
> ==============================================================================
> --- ofbiz/trunk/framework/entity/src/org/ofbiz/entity/model/ModelViewEntity.java (original)
> +++ ofbiz/trunk/framework/entity/src/org/ofbiz/entity/model/ModelViewEntity.java Sun Jun 26 18:10:10 2011
> @@ -1291,7 +1291,7 @@ public class ModelViewEntity extends Mod
> public EntityCondition createCondition(ModelFieldTypeReader modelFieldTypeReader, List<String> entityAliasStack) {
> Object value = this.value;
> // If IN or BETWEEN operator, see if value is a literal list and split it
> - if ((this.operator.equals(EntityOperator.IN) || this.operator.equals(EntityOperator.BETWEEN))
> + if ((this.operator == EntityOperator.IN || this.operator == EntityOperator.BETWEEN)
> && value instanceof String) {
> String delim = null;
> if (((String)value).indexOf("|") >= 0) {
> @@ -1311,7 +1311,7 @@ public class ModelViewEntity extends Mod
> }
>
> // don't convert the field to the desired type if this is an IN or BETWEEN operator and we have a Collection
> - if (!((this.operator.equals(EntityOperator.IN) || this.operator.equals(EntityOperator.BETWEEN))
> + if (!((this.operator == EntityOperator.IN || this.operator == EntityOperator.BETWEEN)
> && value instanceof Collection<?>)) {
> // now to a type conversion for the target fieldName
> value = this.viewEntityCondition.modelViewEntity.convertFieldValue(lhsField, value, modelFieldTypeReader, FastMap.<String, Object>newInstance());
> @@ -1326,7 +1326,7 @@ public class ModelViewEntity extends Mod
> rhs = EntityFieldValue.makeFieldValue(this.relFieldName, this.relEntityAlias, entityAliasStack, this.viewEntityCondition.modelViewEntity);
> }
>
> - if (this.operator.equals(EntityOperator.NOT_EQUAL) && value != null) {
> + if (this.operator == EntityOperator.NOT_EQUAL && value != null) {
> // since some databases don't consider nulls in != comparisons, explicitly include them
> // this makes more sense logically, but if anyone ever needs it to not behave this way we should add an "or-null" attribute that is true by default
> if (ignoreCase) {
> @@ -1340,7 +1340,7 @@ public class ModelViewEntity extends Mod
> EntityOperator.OR,
> EntityCondition.makeCondition(lhs, EntityOperator.EQUALS, null));
> }
> - } else if ( value == null && this.relFieldName == null && (this.operator.equals(EntityOperator.EQUALS) || this.operator.equals(EntityOperator.NOT_EQUAL))) {
> + } else if ( value == null && this.relFieldName == null && (this.operator == EntityOperator.EQUALS || this.operator == EntityOperator.NOT_EQUAL)) {
> return EntityCondition.makeCondition(lhs, this.operator, null);
> } else {
> if (ignoreCase) {
>
>
Re: svn commit: r1139876 - /ofbiz/trunk/framework/entity/src/org/ofbiz/entity/model/ModelViewEntity.java
Posted by Scott Gray <sc...@hotwaxmedia.com>.
It was some openjdk compilation issue, I don't remember the details, people kept complaining so I changed the code. Doesn't really worry me either way, I just recognized the code so sent you an FYI.
Regards
Scott
On 27/06/2011, at 9:36 AM, Adam Heath wrote:
> On 06/26/2011 04:32 PM, Scott Gray wrote:
>> Bit of back and forth regarding == vs. equals: http://svn.apache.org/viewvc?view=revision&revision=810438
>
> But what was the original problem? EntityOperator.FOO are all singletons. Hence the lookupFoo functions. == equality should hold in all cases. If not, it's a bug in the jdk, and I don't think that ofbiz should try to work around that(there can be other == equality tests in the code base that might have similiar issues).
>
>> Regards
>> Scott
>>
>> On 27/06/2011, at 6:10 AM, doogie@apache.org wrote:
>>
>>> Author: doogie
>>> Date: Sun Jun 26 18:10:10 2011
>>> New Revision: 1139876
>>>
>>> URL: http://svn.apache.org/viewvc?rev=1139876&view=rev
>>> Log:
>>> OPTIMIZE: ViewConditionExpr.createCondition: Use == for EntityOperator
>>> equality, instead of equals(); slight speedup, and the code looks a bit nicer.
>>>
>>> Modified:
>>> ofbiz/trunk/framework/entity/src/org/ofbiz/entity/model/ModelViewEntity.java
>>>
>>> Modified: ofbiz/trunk/framework/entity/src/org/ofbiz/entity/model/ModelViewEntity.java
>>> URL: http://svn.apache.org/viewvc/ofbiz/trunk/framework/entity/src/org/ofbiz/entity/model/ModelViewEntity.java?rev=1139876&r1=1139875&r2=1139876&view=diff
>>> ==============================================================================
>>> --- ofbiz/trunk/framework/entity/src/org/ofbiz/entity/model/ModelViewEntity.java (original)
>>> +++ ofbiz/trunk/framework/entity/src/org/ofbiz/entity/model/ModelViewEntity.java Sun Jun 26 18:10:10 2011
>>> @@ -1291,7 +1291,7 @@ public class ModelViewEntity extends Mod
>>> public EntityCondition createCondition(ModelFieldTypeReader modelFieldTypeReader, List<String> entityAliasStack) {
>>> Object value = this.value;
>>> // If IN or BETWEEN operator, see if value is a literal list and split it
>>> - if ((this.operator.equals(EntityOperator.IN) || this.operator.equals(EntityOperator.BETWEEN))
>>> + if ((this.operator == EntityOperator.IN || this.operator == EntityOperator.BETWEEN)
>>> && value instanceof String) {
>>> String delim = null;
>>> if (((String)value).indexOf("|")>= 0) {
>>> @@ -1311,7 +1311,7 @@ public class ModelViewEntity extends Mod
>>> }
>>>
>>> // don't convert the field to the desired type if this is an IN or BETWEEN operator and we have a Collection
>>> - if (!((this.operator.equals(EntityOperator.IN) || this.operator.equals(EntityOperator.BETWEEN))
>>> + if (!((this.operator == EntityOperator.IN || this.operator == EntityOperator.BETWEEN)
>>> && value instanceof Collection<?>)) {
>>> // now to a type conversion for the target fieldName
>>> value = this.viewEntityCondition.modelViewEntity.convertFieldValue(lhsField, value, modelFieldTypeReader, FastMap.<String, Object>newInstance());
>>> @@ -1326,7 +1326,7 @@ public class ModelViewEntity extends Mod
>>> rhs = EntityFieldValue.makeFieldValue(this.relFieldName, this.relEntityAlias, entityAliasStack, this.viewEntityCondition.modelViewEntity);
>>> }
>>>
>>> - if (this.operator.equals(EntityOperator.NOT_EQUAL)&& value != null) {
>>> + if (this.operator == EntityOperator.NOT_EQUAL&& value != null) {
>>> // since some databases don't consider nulls in != comparisons, explicitly include them
>>> // this makes more sense logically, but if anyone ever needs it to not behave this way we should add an "or-null" attribute that is true by default
>>> if (ignoreCase) {
>>> @@ -1340,7 +1340,7 @@ public class ModelViewEntity extends Mod
>>> EntityOperator.OR,
>>> EntityCondition.makeCondition(lhs, EntityOperator.EQUALS, null));
>>> }
>>> - } else if ( value == null&& this.relFieldName == null&& (this.operator.equals(EntityOperator.EQUALS) || this.operator.equals(EntityOperator.NOT_EQUAL))) {
>>> + } else if ( value == null&& this.relFieldName == null&& (this.operator == EntityOperator.EQUALS || this.operator == EntityOperator.NOT_EQUAL)) {
>>> return EntityCondition.makeCondition(lhs, this.operator, null);
>>> } else {
>>> if (ignoreCase) {
>>>
>>>
>>
>
Re: svn commit: r1139876 - /ofbiz/trunk/framework/entity/src/org/ofbiz/entity/model/ModelViewEntity.java
Posted by Adam Heath <do...@brainfood.com>.
On 06/26/2011 04:32 PM, Scott Gray wrote:
> Bit of back and forth regarding == vs. equals: http://svn.apache.org/viewvc?view=revision&revision=810438
But what was the original problem? EntityOperator.FOO are all
singletons. Hence the lookupFoo functions. == equality should hold
in all cases. If not, it's a bug in the jdk, and I don't think that
ofbiz should try to work around that(there can be other == equality
tests in the code base that might have similiar issues).
> Regards
> Scott
>
> On 27/06/2011, at 6:10 AM, doogie@apache.org wrote:
>
>> Author: doogie
>> Date: Sun Jun 26 18:10:10 2011
>> New Revision: 1139876
>>
>> URL: http://svn.apache.org/viewvc?rev=1139876&view=rev
>> Log:
>> OPTIMIZE: ViewConditionExpr.createCondition: Use == for EntityOperator
>> equality, instead of equals(); slight speedup, and the code looks a bit nicer.
>>
>> Modified:
>> ofbiz/trunk/framework/entity/src/org/ofbiz/entity/model/ModelViewEntity.java
>>
>> Modified: ofbiz/trunk/framework/entity/src/org/ofbiz/entity/model/ModelViewEntity.java
>> URL: http://svn.apache.org/viewvc/ofbiz/trunk/framework/entity/src/org/ofbiz/entity/model/ModelViewEntity.java?rev=1139876&r1=1139875&r2=1139876&view=diff
>> ==============================================================================
>> --- ofbiz/trunk/framework/entity/src/org/ofbiz/entity/model/ModelViewEntity.java (original)
>> +++ ofbiz/trunk/framework/entity/src/org/ofbiz/entity/model/ModelViewEntity.java Sun Jun 26 18:10:10 2011
>> @@ -1291,7 +1291,7 @@ public class ModelViewEntity extends Mod
>> public EntityCondition createCondition(ModelFieldTypeReader modelFieldTypeReader, List<String> entityAliasStack) {
>> Object value = this.value;
>> // If IN or BETWEEN operator, see if value is a literal list and split it
>> - if ((this.operator.equals(EntityOperator.IN) || this.operator.equals(EntityOperator.BETWEEN))
>> + if ((this.operator == EntityOperator.IN || this.operator == EntityOperator.BETWEEN)
>> && value instanceof String) {
>> String delim = null;
>> if (((String)value).indexOf("|")>= 0) {
>> @@ -1311,7 +1311,7 @@ public class ModelViewEntity extends Mod
>> }
>>
>> // don't convert the field to the desired type if this is an IN or BETWEEN operator and we have a Collection
>> - if (!((this.operator.equals(EntityOperator.IN) || this.operator.equals(EntityOperator.BETWEEN))
>> + if (!((this.operator == EntityOperator.IN || this.operator == EntityOperator.BETWEEN)
>> && value instanceof Collection<?>)) {
>> // now to a type conversion for the target fieldName
>> value = this.viewEntityCondition.modelViewEntity.convertFieldValue(lhsField, value, modelFieldTypeReader, FastMap.<String, Object>newInstance());
>> @@ -1326,7 +1326,7 @@ public class ModelViewEntity extends Mod
>> rhs = EntityFieldValue.makeFieldValue(this.relFieldName, this.relEntityAlias, entityAliasStack, this.viewEntityCondition.modelViewEntity);
>> }
>>
>> - if (this.operator.equals(EntityOperator.NOT_EQUAL)&& value != null) {
>> + if (this.operator == EntityOperator.NOT_EQUAL&& value != null) {
>> // since some databases don't consider nulls in != comparisons, explicitly include them
>> // this makes more sense logically, but if anyone ever needs it to not behave this way we should add an "or-null" attribute that is true by default
>> if (ignoreCase) {
>> @@ -1340,7 +1340,7 @@ public class ModelViewEntity extends Mod
>> EntityOperator.OR,
>> EntityCondition.makeCondition(lhs, EntityOperator.EQUALS, null));
>> }
>> - } else if ( value == null&& this.relFieldName == null&& (this.operator.equals(EntityOperator.EQUALS) || this.operator.equals(EntityOperator.NOT_EQUAL))) {
>> + } else if ( value == null&& this.relFieldName == null&& (this.operator == EntityOperator.EQUALS || this.operator == EntityOperator.NOT_EQUAL)) {
>> return EntityCondition.makeCondition(lhs, this.operator, null);
>> } else {
>> if (ignoreCase) {
>>
>>
>