Understood. I will revert the two methods.

-Adrian

On 8/14/2012 11:48 PM, Scott Gray wrote:
Taking getBigDecimal for example, where 99.999% of the time the object will 
actually be a BigDecimal, we're now throwing a ClassCastException followed by a 
cast.  I'm not sure how that translates to one object type test?

The Double -> BigDecimal and vice-versa for getDouble are old corner cases left 
over from the conversion to BigDecimal a few years back.

Based on a quick google, the speed difference between throwing an exception vs. 
instanceof seems negligible.  Additionally it is often argued that relying on 
exceptions instead of performing proper checks is considered bad practice 
(hence them being called exceptions).

I don't really mind either way but this just seems like a style preference 
thing rather than an actual optimization.

Regards
Scott

On 14/08/2012, at 9:11 PM, adrian.c...@sandglass-software.com wrote:

The original code had an instanceof and a cast. So, in the case where 
instanceof evaluates to true, there will be two object type tests - instanceof 
and the cast. The modification guarantees there will be only one object type 
test.

I've seen this optimization used elsewhere, but I haven't actually measured it.

-Adrian

Quoting Scott Gray <scott.g...@hotwaxmedia.com>:

Hi Adrian,

Are you sure those are optimizations?  For getBigDecimal and getDouble I think 
almost every call would cause the exception to be thrown, is that actually more 
efficient than an instanceof check?  I know both are expensive but I don't know 
which is more so.

Thanks
Scott

On 14/08/2012, at 6:02 PM, adri...@apache.org wrote:

Author: adrianc
Date: Tue Aug 14 06:02:58 2012
New Revision: 1372738

URL: http://svn.apache.org/viewvc?rev=1372738&view=rev
Log:
Small GenericEntity optimization - remove instanceof checks in getXxx methods.

Modified:
   ofbiz/trunk/framework/entity/src/org/ofbiz/entity/GenericEntity.java

Modified: ofbiz/trunk/framework/entity/src/org/ofbiz/entity/GenericEntity.java
URL: 
http://svn.apache.org/viewvc/ofbiz/trunk/framework/entity/src/org/ofbiz/entity/GenericEntity.java?rev=1372738&r1=1372737&r2=1372738&view=diff
==============================================================================
--- ofbiz/trunk/framework/entity/src/org/ofbiz/entity/GenericEntity.java 
(original)
+++ ofbiz/trunk/framework/entity/src/org/ofbiz/entity/GenericEntity.java Tue 
Aug 14 06:02:58 2012
@@ -619,14 +619,8 @@ public class GenericEntity extends Obser
    }

    public String getString(String name) {
-        // might be nice to add some ClassCastException handling... and auto 
conversion? hmmm...
        Object object = get(name);
-        if (object == null) return null;
-        if (object instanceof java.lang.String) {
-            return (String) object;
-        } else {
-            return object.toString();
-        }
+        return object == null ? null : object.toString();
    }

    public java.sql.Timestamp getTimestamp(String name) {
@@ -656,22 +650,28 @@ public class GenericEntity extends Obser
    public Double getDouble(String name) {
        // this "hack" is needed for now until the Double/BigDecimal issues are 
all resolved
        Object value = get(name);
-        if (value instanceof BigDecimal) {
-            return Double.valueOf(((BigDecimal) value).doubleValue());
-        } else {
-            return (Double) value;
+        if (value != null) {
+            try {
+                BigDecimal numValue = (BigDecimal) value;
+                return new Double(numValue.doubleValue());
+            } catch (ClassCastException e) {
+            }
        }
+        return (Double) value;
    }

    public BigDecimal getBigDecimal(String name) {
        // this "hack" is needed for now until the Double/BigDecimal issues are 
all resolved
        // NOTE: for things to generally work properly BigDecimal should really 
be used as the java-type in the field type def XML files
        Object value = get(name);
-        if (value instanceof Double) {
-            return BigDecimal.valueOf(((Double) value).doubleValue());
-        } else {
-            return (BigDecimal) value;
+        if (value != null) {
+            try {
+                Double numValue = (Double) value;
+                return new BigDecimal(numValue.doubleValue());
+            } catch (ClassCastException e) {
+            }
        }
+        return (BigDecimal) value;
    }

    @SuppressWarnings("deprecation")






Reply via email to