Hi Martin, sorry for the late reply. My comments are inline:
Martin Desruisseaux wrote:
>Graham Davis a écrit :
>
>
>>public boolean equals(Object o) {
>> if (o instanceof DirectPosition || o instanceof DirectPositionImpl)
>> return this.equals((DirectPosition) o, 0);
>> else
>> return false;
>>}
>>
>>
>
>Note: the "|| o instanceof DirectPositionImpl" is useless. I suggest to remove
>it.
>
>
Good eye, that does seem useless. I will remove that.
>>/**
>> * Compares coodinates of Direct Positions and allows a tolerance value in
>> * the comparison. Implementation Note: Parameter has to be of Type
>> * DirectPosition (not DirectPositionImpl), so that the equals method is
>> * found for DirectPosition´s and DirectPositionImpl´s
>>
>>
>
>I suggest to remove the implementation note: it is normal interface programming
>and don't really bring info as far as I can see...
>
>
>
Good point, I will remove this too.
>>public boolean equals(DirectPosition position, double tol) {
>> int D = position.getDimension();
>> if( D != getDimension() ) return false;
>> for (int i = 0; i < D; ++i) {
>> if (Math.abs(DoubleOperation.subtract(position.getOrdinate(i),
>> this.coordinate[i])) > tol)
>> return false;
>> }
>> return true;
>>}
>>
>>
>
>What is DoubleOperation? What are its advantage compared to the usual '-'
>operator? position.getOrdinate(i) returns double value anyway, and
>Math.abs(double) expect a double anyway.
>
>Additional note: I suggest to replace:
>
> if (abs(position.getOrdinate(i) - this.getOrdinate(i)) > tol)
>
>by
>
> if (!(abs(position.getOrdinate(i) - this.getOrdinate(i))) <= tol)
>
>Purpose: consider positions as different if at least one ordinate is
>Double.NaN.
>
> Martin
>
>
I'm not sure why the original implementer uses DoubleOperation here. I
assumed it was to catch special cases when one or both values were NaN,
but looking at the code, it just seems to use the regular '-' operator.
Your suggestion again seems good, and I will implement this change as well.
Thanks for the feedback Martin!
Graham.
--
Graham Davis
Refractions Research Inc.
[EMAIL PROTECTED]
-------------------------------------------------------------------------
This SF.net email is sponsored by DB2 Express
Download DB2 Express C - the FREE version of DB2 express and take
control of your XML. No limits. Just data. Click to get it now.
http://sourceforge.net/powerbar/db2/
_______________________________________________
Geotools-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/geotools-devel
