[MATH-387] Duplicate code

• Type: Improvement
• Status: Closed
• Priority: Minor
• Resolution: Fixed
• Affects Version/s: 2.1
• Fix Version/s: 2.2
• Assignee: Unassigned
• Reporter: Gilles Sadowski
• Created: 16/Jul/10 14:55
• Updated: 23/Mar/11 20:12
• Resolved: 19/Jul/10 14:59

Description

In package optimization:

SimpleRealPointChecker.java

public boolean converged(final int iteration, final RealPointValuePair previous, final RealPointValuePair current) {
    final double[] p        = previous.getPoint();
    final double[] c        = current.getPoint();
    for (int i = 0; i < p.length; ++i) {
        final double difference = Math.abs(p[i] - c[i]);
        final double size       = Math.max(Math.abs(p[i]), Math.abs(c[i]));
        if ((difference > (size * relativeThreshold)) && (difference > absoluteThreshold)) {
            return false;
        }
    }
    return true;
}

SimpleVectorialPointChecker.java

public boolean converged(final int iteration, final VectorialPointValuePair previous, final VectorialPointValuePair current) {
    final double[] p = previous.getPointRef();
    final double[] c = current.getPointRef();
    for (int i = 0; i < p.length; ++i) {
        final double pi         = p[i];
        final double ci         = c[i];
        final double difference = Math.abs(pi - ci);
        final double size       = Math.max(Math.abs(pi), Math.abs(ci));
        if ((difference > (size * relativeThreshold)) &&
            (difference > absoluteThreshold)) {
            return false;
        }
    }
    return true;
}

Do they do the same thing or am I missing something?

Also in

SimpleScalarValueChecker.java

public boolean converged(final int iteration, final RealPointValuePair previous, final RealPointValuePair current) {
    final double p          = previous.getValue();
    final double c          = current.getValue();
    final double difference = Math.abs(p - c);
    final double size       = Math.max(Math.abs(p), Math.abs(c));
    return (difference <= (size * relativeThreshold)) || (difference <= absoluteThreshold);
}

it seems overkill that one must create two RealPointValuePair objects when one just wants to compare two double. Shouldn't this class contain a method like

public boolean converged(int iteration, double previous, double current) {
    final double difference = Math.abs(previous - current);
    final double size       = Math.max(Math.abs(previous), Math.abs(current));
    return (difference <= (size * relativeThreshold)) || (difference <= absoluteThreshold);

?

Also none of these methods seem to need an iteration parameter.