Hi Andrea, Seems like this is a problem that has been solved on trunk as well. I understand that the changes there are not back portable to 2.3.x. However we should ensure that the same behaviour is maintained.
I can't spot any problems with the routine you wrote. Perhaps the only thing is that it doesn't seem to handle BigDecimal or BigInteger. -Justin Andrea Aime wrote: > Hi all, > I've found various issues in IsEqualsToImpl as > found in 2.3.x. The original implementation is attached, > as well as a new implementation that should handle properly > most numeric cases. > The original implementation had various issues due to > numeric overflows and decimal part truncations, and could > not compare properly 5 and 5.0, for example. > > I've tried to build a better one, that I've attached > along with the current one. > Since this is a very central part of geotools I'd like > to have a review or two before committing. > > Cheers > Andrea > > > > > > !DSPAM:1004,458805cf286012051017194! > > > ------------------------------------------------------------------------ > > /* > * GeoTools - OpenSource mapping toolkit > * http://geotools.org > * (C) 2006, GeoTools Project Managment Committee (PMC) > * > * This library is free software; you can redistribute it and/or > * modify it under the terms of the GNU Lesser General Public > * License as published by the Free Software Foundation; > * version 2.1 of the License. > * > * This library is distributed in the hope that it will be useful, > * but WITHOUT ANY WARRANTY; without even the implied warranty of > * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU > * Lesser General Public License for more details. > */ > package org.geotools.filter; > > import java.util.Arrays; > import java.util.HashSet; > import java.util.Set; > > import org.geotools.feature.Feature; > import org.opengis.filter.FilterVisitor; > import org.opengis.filter.PropertyIsEqualTo; > import org.opengis.filter.expression.Expression; > > /** > * > * @author jdeolive > */ > public class IsEqualsToImpl extends CompareFilterImpl implements > PropertyIsEqualTo { > > private static Set SUPPORTED_NUMERIC = new HashSet(Arrays.asList(new > Class[] { Byte.class, > Short.class, Integer.class, Long.class, Float.class, Double.class > })); > > protected IsEqualsToImpl(FilterFactory factory) { > this(factory, null, null); > } > > protected IsEqualsToImpl(FilterFactory factory, Expression expression1, > Expression expression2) { > super(factory, expression1, expression2); > > // backwards compat with old type system > this.filterType = COMPARE_EQUALS; > } > > // @Override > public boolean evaluate(Feature feature) { > Object value1 = eval(expression1, feature); > Object value2 = eval(expression2, feature); > > if (value1 == null && value2 == null) > return true; > if (value1 == null && value2 != null || value1 != null && value2 == > null) > return false; > > if (value1.getClass().equals(value2.getClass())) > return value1.equals(value2); > > if ((SUPPORTED_NUMERIC.contains(value1.getClass()) && > SUPPORTED_NUMERIC.contains(value2.getClass())) || > (SUPPORTED_NUMERIC.contains(value1.getClass()) && value2 instanceof > CharSequence) || > (SUPPORTED_NUMERIC.contains(value2.getClass()) && value1 instanceof > CharSequence)) { > // numeric comparison, try to parse strings to numbers and do > proper > // comparison between, say, 5 and 5.0 (Long and Double would say > // they are different) > try { > if (value1 instanceof CharSequence) > value1 = parseToNumber((CharSequence) value1); > if (value2 instanceof CharSequence) > value2 = parseToNumber((CharSequence) value1); > > } catch (java.lang.NumberFormatException e) { > // the string cannot be cast to number, so it's different > return false; > } > > value1 = promote(value1); > value2 = promote(value2); > > // hard case, both numbers, but not same type. equals would fail > if(!value1.getClass().equals(value2.getClass())) { > // one is a double, the other is a long > double d = 0; > long l = 0; > if(value1 instanceof Long) { > l = ((Long) value1).longValue(); > d = ((Double) value2).doubleValue(); > } else { > l = ((Long) value2).longValue(); > d = ((Double) value1).doubleValue(); > } > > if((d - Math.floor(d)) != 0) > return false; // not integral > > if(d > Long.MAX_VALUE || d < Long.MIN_VALUE) > return false; // out of range > > // then the double can be represented as a long without loss > // compare them directly > return ((long) d) == l; > } > } > return value1.equals(value2); > } > > protected Object parseToNumber(CharSequence value) { > // In CharSequence toString is guaranteed to return a string > equivalent > // of the sequence > String s = value.toString(); > try { > return new Long(Long.parseLong(s)); > } catch (NumberFormatException e) { > return new Double(Double.parseDouble(s)); > } > } > > protected Object promote(Object o) { > if (o instanceof Short || o instanceof Integer || o instanceof Byte) > return new Long(((Number) o).longValue()); > if (o instanceof Float) > return new Double(((Number) o).floatValue()); > return o; > } > > public Object accept(FilterVisitor visitor, Object extraData) { > return visitor.visit(this, extraData); > } > } > > > !DSPAM:1004,458805cf286012051017194! > > > ------------------------------------------------------------------------ > > /* > * GeoTools - OpenSource mapping toolkit > * http://geotools.org > * (C) 2006, GeoTools Project Managment Committee (PMC) > * > * This library is free software; you can redistribute it and/or > * modify it under the terms of the GNU Lesser General Public > * License as published by the Free Software Foundation; > * version 2.1 of the License. > * > * This library is distributed in the hope that it will be useful, > * but WITHOUT ANY WARRANTY; without even the implied warranty of > * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU > * Lesser General Public License for more details. > */ > package org.geotools.filter; > > import java.util.Arrays; > import java.util.HashSet; > import java.util.Set; > > import org.geotools.feature.Feature; > import org.opengis.filter.FilterVisitor; > import org.opengis.filter.PropertyIsEqualTo; > import org.opengis.filter.expression.Expression; > > /** > * > * @author jdeolive > */ > public class IsEqualsToImpl extends CompareFilterImpl implements > PropertyIsEqualTo { > > private static Set SUPPORTED_NUMERIC = new HashSet(Arrays.asList(new > Class[] { Byte.class, > Short.class, Integer.class, Long.class, Float.class, Double.class > })); > > protected IsEqualsToImpl(FilterFactory factory) { > this(factory, null, null); > } > > protected IsEqualsToImpl(FilterFactory factory, Expression expression1, > Expression expression2) { > super(factory, expression1, expression2); > > // backwards compat with old type system > this.filterType = COMPARE_EQUALS; > } > > // @Override > public boolean evaluate(Feature feature) { > Object value1 = eval(expression1, feature); > Object value2 = eval(expression2, feature); > > if (value1 == null && value2 == null) > return true; > if (value1 == null && value2 != null || value1 != null && value2 == > null) > return false; > > if (value1.getClass().equals(value2.getClass())) > return value1.equals(value2); > > if ((SUPPORTED_NUMERIC.contains(value1.getClass()) && > SUPPORTED_NUMERIC.contains(value2.getClass())) || > (SUPPORTED_NUMERIC.contains(value1.getClass()) && value2 instanceof > CharSequence) || > (SUPPORTED_NUMERIC.contains(value2.getClass()) && value1 instanceof > CharSequence)) { > // numeric comparison, try to parse strings to numbers and do > proper > // comparison between, say, 5 and 5.0 (Long and Double would say > // they are different) > try { > if (value1 instanceof CharSequence) > value1 = parseToNumber((CharSequence) value1); > if (value2 instanceof CharSequence) > value2 = parseToNumber((CharSequence) value1); > > } catch (java.lang.NumberFormatException e) { > // the string cannot be cast to number, so it's different > return false; > } > > value1 = promote(value1); > value2 = promote(value2); > > // hard case, both numbers, but not same type. equals would fail > if(!value1.getClass().equals(value2.getClass())) { > // one is a double, the other is a long > double d = 0; > long l = 0; > if(value1 instanceof Long) { > l = ((Long) value1).longValue(); > d = ((Double) value2).doubleValue(); > } else { > l = ((Long) value2).longValue(); > d = ((Double) value1).doubleValue(); > } > > if((d - Math.floor(d)) != 0) > return false; // not integral > > if(d > Long.MAX_VALUE || d < Long.MIN_VALUE) > return false; // out of range > > // then the double can be represented as a long without loss > // compare them directly > return ((long) d) == l; > } > } > return value1.equals(value2); > } > > protected Object parseToNumber(CharSequence value) { > // In CharSequence toString is guaranteed to return a string > equivalent > // of the sequence > String s = value.toString(); > try { > return new Long(Long.parseLong(s)); > } catch (NumberFormatException e) { > return new Double(Double.parseDouble(s)); > } > } > > protected Object promote(Object o) { > if (o instanceof Short || o instanceof Integer || o instanceof Byte) > return new Long(((Number) o).longValue()); > if (o instanceof Float) > return new Double(((Number) o).floatValue()); > return o; > } > > public Object accept(FilterVisitor visitor, Object extraData) { > return visitor.visit(this, extraData); > } > } > > > !DSPAM:1004,458805cf286012051017194! > > > ------------------------------------------------------------------------ > > ------------------------------------------------------------------------- > Take Surveys. Earn Cash. Influence the Future of IT > Join SourceForge.net's Techsay panel and you'll get the chance to share your > opinions on IT & business topics through brief surveys - and earn cash > http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV > > !DSPAM:1004,458805cf286012051017194! > > > ------------------------------------------------------------------------ > > _______________________________________________ > Geotools-devel mailing list > [email protected] > https://lists.sourceforge.net/lists/listinfo/geotools-devel > > > !DSPAM:1004,458805cf286012051017194! -- Justin Deoliveira [EMAIL PROTECTED] The Open Planning Project http://topp.openplans.org ------------------------------------------------------------------------- Take Surveys. Earn Cash. Influence the Future of IT Join SourceForge.net's Techsay panel and you'll get the chance to share your opinions on IT & business topics through brief surveys - and earn cash http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV _______________________________________________ Geotools-devel mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/geotools-devel
