Andrea Aime a écrit :
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.
I submit there an other proposal. The previous one had issues with the usual
corner cases (-0.0 vs +0.0 which are not equals according Number.equals(Object),
NaN values...). This proposal also try to avoid some redundancies (e.g. looking
twice the same object in the same hashset) and try to work with arbitrary Number
as long as they are representable as long or double primitive type (so we do not
cover BigInteger / BigDecimal case, but at least we get ride of list of special
cases like SUPPORTED_NUMERIC hashset).
In this proposal, the 'promote' method disaspear and 'parseToNumber' is made
private. I don't know when those method were introduced, so I don't know if we
are allowed to hide them.
Martin
/*
* 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 org.geotools.feature.Feature;
import org.opengis.filter.FilterVisitor;
import org.opengis.filter.PropertyIsEqualTo;
import org.opengis.filter.expression.Expression;
/**
* Compares values. This filter treat subclasses of [EMAIL PROTECTED] Number}
in a special case: numbers
* are compared as [EMAIL PROTECTED] long} (if possible) or [EMAIL PROTECTED]
double} (otherwise) primitive types no
* matter what the classes are ([EMAIL PROTECTED] Short}, [EMAIL PROTECTED]
Integer}, [EMAIL PROTECTED] Float}, <cite>etc.</cite>).
* This process allows the comparaison of numbers of different types. It work
for standard types
* in the [EMAIL PROTECTED] java.lang} package as well as user-specific [EMAIL
PROTECTED] Number} subclasses, as long as
* they are representable as [EMAIL PROTECTED] long} or [EMAIL PROTECTED]
double} primitive types.
* <p>
* Current implementation will <strong>not</strong> work reliably for [EMAIL
PROTECTED] java.math.BigInteger}
* or [EMAIL PROTECTED] java.math.BigDecimal} types, since their value may be
outside the [EMAIL PROTECTED] long} and
* [EMAIL PROTECTED] double} ranges.
*
* @source $URL$
* @version $Id$
* @author Justin Deolive
* @author Andrea Aimes
*/
public class IsEqualsToImpl extends CompareFilterImpl implements
PropertyIsEqualTo {
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(final Feature feature) {
final Object value1 = eval(expression1, feature);
final Object value2 = eval(expression2, feature);
if (value1 == value2) {
// Includes the (value1 == null && value2 == null) case.
return true;
}
if (value1 == null || value2 == null) {
// No need to check for (value2 != null) or (value1 != null).
// If they were null, the previous check would have caugh them.
return false;
}
/*
* For the following "if (value1.equals(value2))" line, we do not
* check for "value1.getClass().equals(value2.getClass())" because:
*
* 1) It is usually done inside the 'Object.equals(Object)' method,
* so our check would be redundant.
* 2) It would lead to a subtile issue with the 0.0 floating point
* value, as explained below.
*
* Even if 'value1' and 'value2' are of the same class, we can not
implement this code as
* "return value1.equals(value2)" because a 'false' value doesn't means
that the values
* are numerically different. Float and Double.equals(Object) return
'false' when comparing
* +0.0 with -0.0, while +0.0 == -0.0 returns 'true'. We would need to
make a special case
* for them. It is better to fallback on the more generic code
following the "if" block in
* order to ensure consistent behavior.
*/
if (value1.equals(value2)) {
return true;
}
final boolean isNumeric1 = (value1 instanceof Number);
final boolean isNumeric2 = (value2 instanceof Number);
if ((isNumeric1 || (value1 instanceof CharSequence)) &&
(isNumeric2 || (value2 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)
final Number n1, n2;
try {
n1 = isNumeric1 ? (Number) value1 :
parseToNumber(value1.toString());
n2 = isNumeric2 ? (Number) value2 :
parseToNumber(value2.toString());
} catch (NumberFormatException e) {
// The string cannot be cast to number, so it's different.
return false;
}
final double fp1 = n1.doubleValue();
final double fp2 = n2.doubleValue();
final long lg1, lg2; // 'lg2' will not be initialized if not needed.
if (fp1 == (double) (lg1 = n1.longValue()) &&
fp2 == (double) (lg2 = n2.longValue()))
{
// Compares the values as 'long' if and only if the 'double'
values
// do not contains any additional informations.
return lg1 == lg2;
} else {
// Floating point comparaisons. Note: we do NOT use
Double.equals or
// Double.doubleToLongBits because we want to consider +0.0 ==
-0.0.
// The Double.equals method would returns 'false' in the above
case.
return (fp1 == fp2) || (Double.isNaN(fp1) && Double.isNaN(fp2));
}
}
return value1.equals(value2);
}
/**
* Parses the specified string as a [EMAIL PROTECTED] Long} or a [EMAIL
PROTECTED] Double} value.
*
* @param value The string to parse.
* @return The value as a number.
* @throws NumberFormatException if the string can't be parsed.
*/
private static Number parseToNumber(final String value) throws
NumberFormatException {
try {
return Long.valueOf(value);
} catch (NumberFormatException e) {
return Double.valueOf(value);
}
}
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