[
https://issues.apache.org/jira/browse/NUMBERS-77?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17327876#comment-17327876
]
Alex Herbert commented on NUMBERS-77:
-------------------------------------
Looking at Matt's PR I would agree that there is no need for the
AbstractPrecisionComparator when all the methods can be default methods in the
interface. I like the implementation from Gilles with a simple interface with
default methods.
All the comparisons in {{sign}} seem redundant:
{code:java}
default int sign(double a) {
final int cmp = compare(a, 0.0);
if (cmp < 0) {
return -1;
} else if (cmp > 0) {
return 1;
}
return 0;
}
{code}
The only reason for all the comparisons is if you expect compare to return a
value that is not in [-1, 0, 1]. If so then the implementation of compare is
wrong. Surely it would be:
{code:java}
default int sign(double a) {
return compare(a, 0.0);
}
{code}
The question is do you wish it to return -1 for -0.0? Using Double.compare(a,
0.0) as the implementation it would do. Using the operators < and > it would
not. Any use of an epsilon should allow it.
So you have to deal with the use of -0.0 and 0.0 and what to expect. This would
be a signum function that handles -0.0 and 0.0 as returning 0.0:
{code:java}
default double signum(double a) {
return (a == 0.0 || compare(a, 0.0) == 0 || Double.isNaN(d)) ? d :
Math.copySign(1.0, d);
}
{code}
> Move utilities from "Commons Geometry"
> --------------------------------------
>
> Key: NUMBERS-77
> URL: https://issues.apache.org/jira/browse/NUMBERS-77
> Project: Commons Numbers
> Issue Type: Task
> Reporter: Gilles Sadowski
> Priority: Major
> Fix For: 1.1
>
> Attachments: NUMBERS-77.diff
>
> Time Spent: 1h
> Remaining Estimate: 0h
>
> "Commons Geometry" defines utilities that would be a better fit in this
> component.
> Duplication of general-purpose codes should be avoided, in order to benefit
> from consolidated usage (bug reporting, performance enhancements, ...).
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
