Thanks Adrian. I wonder when we'd be safe to remove those checks altogether.
Regards Scott On 15/08/2012, at 6:25 PM, Adrian Crum wrote: > 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") >>>>> >>>>> >>>> >>> >>> >
