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
/*
* 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);
}
}
/*
* 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);
}
}
-------------------------------------------------------------------------
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
