8
\$\begingroup\$

This is a follow up post for this question.

There aren't a lot of changes. The changes (all of them major) are:

  1. I've decided to ditch my previous approach using Math.ulp() and adopt Tim Leek's suggestion: comparing ratios. In my updated code, I calculate both ratios and ensure that their difference from 1 is within the tolerance.
  2. The flexibility of the class is still kept in mind; getters and setters have been added to compliment the private static tolerance values.
  3. The constructor is made private to prevent instantiation (which I simply forgot about in the previous version).

Basically, I fix my code when someone manages to break it. Tom Leek opened my eyes. Spotted's answer was nice too, but it's too bad that it must be done in a slightly different way to ensure compilation. It was a nice feeling when I saw my code pass in the cases mentioned by Tom Leek.

package library.util;

/**
 * This class provides a useful interface for easy (approximate) floating
 * point equality checks. This is necessary for several other classes. This
 * class provides the user the power to set custom values for the
 * <i>tolerances</i> used in comparison.
 *
 * @author Subhomoy Haldar (ambigram_maker)
 * @version 1.5
 */
public class Numbers {

    /**
     * Private constructor to prevent instantiation.
     */
    private Numbers() {
        throw new AssertionError("No instances for you! ;P");
    }

    /**
     * The default tolerance value for comparing the {@code float} values.
     */
    public static final float DEFAULT_FLOAT_TOLERANCE = 5E-8f;
    /**
     * The default tolerance value for comparing the {@code double} values.
     */
    public static final double DEFAULT_DOUBLE_TOLERANCE = 5E-16;

    /**
     * The current tolerance to be used for comparing floats.
     */
    private static float floatTolerance = DEFAULT_FLOAT_TOLERANCE;
    /**
     * The current tolerance to be used for comparing doubles.
     */
    private static double doubleTolerance = DEFAULT_DOUBLE_TOLERANCE;

    /**
     * Returns the current tolerance value for comparing {@code float}s.
     *
     * @return The current tolerance value for comparing {@code float}s.
     */
    public static float getFloatTolerance() {
        return floatTolerance;
    }

    /**
     * This method allows the user to set a custom value for the tolerance for
     * comparing {@code float}s. The value must always be positive.
     *
     * @param floatTolerance The new tolerance value.
     * @throws IllegalArgumentException If the value given is absurd.
     */
    public static void setFloatTolerance(float floatTolerance) {
        if (floatTolerance <= 0) {
            throw new IllegalArgumentException("Absurd tolerance value.");
        }
        Numbers.floatTolerance = floatTolerance;
    }

    /**
     * Returns the current tolerance value for comparing {@code double}s.
     *
     * @return The current tolerance value for comparing {@code double}s.
     */
    public static double getDoubleTolerance() {
        return doubleTolerance;
    }

    /**
     * This method allows the user to set a custom value for the tolerance for
     * comparing {@code double}s. The value must always be positive.
     *
     * @param doubleTolerance The new tolerance value.
     * @throws IllegalArgumentException If the value given is absurd.
     */
    public static void setDoubleTolerance(double doubleTolerance) {
        if (doubleTolerance <= 0) {
            throw new IllegalArgumentException("Absurd tolerance value.");
        }
        Numbers.doubleTolerance = doubleTolerance;
    }

    /**
     * Returns {@code true} if the arguments are <i>exactly</i> equal.
     *
     * @param bytes The arguments to check.
     * @return {@code true} if the arguments are <i>exactly</i> equal.
     */
    public static boolean areEqual(byte... bytes) {
        int length = bytes.length;
        assertProperLength(length);
        byte d = bytes[0];
        for (int i = 1; i < length; i++) {
            if (d != bytes[i]) {
                return false;
            }
        }
        return true;
    }

    /**
     * Returns {@code true} if the arguments are <i>exactly</i> equal.
     *
     * @param shorts The arguments to check.
     * @return {@code true} if the arguments are <i>exactly</i> equal.
     */
    public static boolean areEqual(short... shorts) {
        int length = shorts.length;
        assertProperLength(length);
        short d = shorts[0];
        for (int i = 1; i < length; i++) {
            if (d != shorts[i]) {
                return false;
            }
        }
        return true;
    }

    /**
     * Returns {@code true} if the arguments are <i>exactly</i> equal.
     *
     * @param ints The arguments to check.
     * @return {@code true} if the arguments are <i>exactly</i> equal.
     */
    public static boolean areEqual(int... ints) {
        int length = ints.length;
        assertProperLength(length);
        int d = ints[0];
        for (int i = 1; i < length; i++) {
            if (d != ints[i]) {
                return false;
            }
        }
        return true;
    }

    /**
     * Returns {@code true} if the arguments are <i>exactly</i> equal.
     *
     * @param longs The arguments to check.
     * @return {@code true} if the arguments are <i>exactly</i> equal.
     */
    public static boolean areEqual(long... longs) {
        int length = longs.length;
        assertProperLength(length);
        long d = longs[0];
        for (int i = 1; i < length; i++) {
            if (d != longs[i]) {
                return false;
            }
        }
        return true;
    }

    /**
     * Returns {@code true} if the arguments are <i>exactly</i> equal.
     *
     * @param chars The arguments to check.
     * @return {@code true} if the arguments are <i>exactly</i> equal.
     */
    public static boolean areEqual(char... chars) {
        int length = chars.length;
        assertProperLength(length);
        char d = chars[0];
        for (int i = 1; i < length; i++) {
            if (d != chars[i]) {
                return false;
            }
        }
        return true;
    }

    /**
     * Returns {@code true} if the arguments are <i>approximately</i> equal.
     *
     * @param floats The arguments to check.
     * @return {@code true} if the arguments are <i>approximately</i> equal.
     * @see #areEqual(double, double) areEqual(double, double) -
     * For an important note.
     */
    public static boolean areEqual(float... floats) {
        int length = floats.length;
        assertProperLength(length);
        float d = floats[0];
        for (int i = 1; i < length; i++) {
            if (!areEqual(d, floats[i])) {
                return false;
            }
        }
        return true;
    }

    /**
     * Returns {@code true} if the arguments are <i>approximately</i> equal.
     * <p>
     * <b>NOTE:</b> There are a few details which must be explained:
     * <ol><li>When all the arguments are <i>infinities</i> (of the same sign),
     * then this method returns {@code true}.</li>
     * <li>When all the arguments are {@link Double#NaN Double.NaN}, then
     * this method returns {@code true}.</li></ol>
     * This goes <i>against</i> the standard specification (which requires that
     * &infin; &ne; &infin; and {@code NaN} &ne; {@code NaN}). This is
     * <i>counter-intuitive</i> and that is why this implementation does not
     * follow the specification. The same applies for the
     * {@link #areEqual(float, float) areEqual(float, float)} method.
     *
     * @param doubles The arguments to check.
     * @return {@code true} if the arguments are <i>approximately</i> equal.
     */
    public static boolean areEqual(double... doubles) {
        int length = doubles.length;
        assertProperLength(length);
        double d = doubles[0];
        for (int i = 1; i < length; i++) {
            if (!areEqual(d, doubles[i])) {
                return false;
            }
        }
        return true;
    }

    /**
     * This method is used to ensure that the number of arguments is at least 2.
     *
     * @param length The number of arguments.
     * @throws IllegalArgumentException If 1 or no arguments.
     */
    private static void assertProperLength(int length) throws
            IllegalArgumentException {
        if (length < 2) {
            throw new IllegalArgumentException
                    ("At least two arguments required.");
        }
    }

    private static boolean areEqual(double d1, double d2) {
        // the corner cases first:
        if (Double.isNaN(d1) && Double.isNaN(d2)) {
            return true;
        }
        if (Double.isInfinite(d1) || Double.isInfinite(d2)) {
            return d1 == d2;    // ensures same sign
        }
        if (d1 == 0 || d2 == 0) {
            return Math.abs(d1 - d2) <= doubleTolerance;
        }
        // No more "kludgy business with Math.ulp()." ;)
        // Just compare the ratios:
        double ratio1 = d1 / d2;    // Two ratios are there
        double ratio2 = d2 / d1;    // for symmetry.
        return d1 == d2 ||
                Math.abs(ratio1 - 1) <= doubleTolerance &&
                Math.abs(ratio2 - 1) <= doubleTolerance;
    }

    private static boolean areEqual(float f1, float f2) {
        if (Float.isNaN(f1) && Float.isNaN(f2)) {
            return true;
        }
        if (Float.isInfinite(f1) || Float.isInfinite(f2)) {
            return f1 == f2;
        }
        if (f1 == 0 || f2 == 0) {
            return Math.abs(f1 - f2) <= floatTolerance;
        }
        double ratio1 = f1 / f2;
        double ratio2 = f2 / f1;
        return f1 == f2 ||
                Math.abs(ratio1 - 1) <= floatTolerance &&
                Math.abs(ratio2 - 1) <= floatTolerance;
    }
}

Now, for the cases mentioned by Tom Leek:

double a = 4.6e4;   // any arbitrary value
double b = a + 0.9 * Numbers.getDoubleTolerance();
double c = a - 0.9 * Numbers.getDoubleTolerance();
assert Numbers.areEqual(a, b, c);
assert Numbers.areEqual(b, a, c);
assert !Numbers.areEqual(1e-30, 1e-36);
double d = Math.pow(2, 53);
double e = d + 1;
assert d == e;
assert Numbers.areEqual(d, e);

The above code runs without resulting in any assertion errors. My request is simple: try breaking this one as well. ;-)

Also, I would like the usual: suggestions on improving readability, reusability and so on.

\$\endgroup\$
  • 1
    \$\begingroup\$ areEqual should be areNearlyEqual since, as pointed out in a response to the earlier question, this version of equality is not transitive, hence, hides a trap. \$\endgroup\$ – Pete Becker Sep 7 '15 at 15:21
8
\$\begingroup\$
  1. Please make the class final to let noone inherit the class.
    1. Nobody would expect a AsserationError if he can inherit the class.
    2. Everyone who inherit the class would have duplicate javadoc.
  2. Please throw a RuntimeException or an IllegalStateException because the javadoc matches better.
  3. ;P let me miss seriousness.
  4. i miss strictfp at the class definition because the math-processor may wrong.
  5. getFloatTolerance you dont need the getter. If it would be a bean, you need it but it is no bean.
  6. setFloatTolerance throws a IAE, but this behaviour is not part of the javadoc, and if you add this behaviour to the javadoc you shall write the signature of the method matching to the javadoc. assertProperLength should be have the correct javadoc and signature too (including the pending methods).
  7. The classname Numbers should be named "NumberHelper".
  8. The above code runs without resulting in any assertion errors. (Yipee!!), Yipee but wait ... did you run it as JUnit-Tests or activated the -ea by manual?
  9. Some methods needs to have javadoc.

I have a few more but ill stop here because this class is not thread-save.

\$\endgroup\$
  • \$\begingroup\$ Forgot to add final to prevent inheritance as it is a library. Good catch. Also, strictfp would be a good addition. You are right about updating the signatures. But I think Numbers.areEqual(...) is preferable to NumberHelper.areEqual(...). I originally tested using JUnit4. I modified the code and presented it here. I don't think private methods need JavaDoc if they are readable enough. You should probably take a look at the previous post. \$\endgroup\$ – Hungry Blue Dev Sep 7 '15 at 15:08
  • 1
    \$\begingroup\$ About the naming ... yes, its a dispute. As you may notice the java.util.Objects (who use similar naming like your Numbers) came from Oracle or Sun. But StringUtils (who use similar naming like my NumberUtils came from ASF+Friends who are much more near to the Projects than the JDK ever will be. Red or Blue pill neo. \$\endgroup\$ – Peter Rader Sep 8 '15 at 7:23
3
\$\begingroup\$

Generic methods

You can use generic methods for something like this. Rather than having to type out methods for each individual integer type, you can use a generic method for your areEqual methods, like this:

public static <TNumber> boolean areEqual(TNumber... numbers) {
    int length = numbers.length;
    assertProperLength(length);
    TNumber d = numbers[0];        
    for (int i = 1; i < length; i++) {
        if (!d.equals(numbers[i])) {
            return false;
        }
    }
    return true;
}

This eliminates the need to create duplicate methods for each individual type, and makes your code generally easier to read.

This can also apply to your areEqual overloads with just two arguments. I wasn't quite sure how to implement them in this context, although I'm quite sure you can figure it out. Be sure to use .equal though rather than ==, as areEqual may return false when true is expected.


Nitpicks

Some of your error messages are not so great. For example:

  • "No instances for you! ;P" would be better as "An instance of this class cannot be created.".
  • "Absurd tolerance value." would also be better as "Invalid tolerance value. Tolerance value must be ...".

The error types that you're raising don't make much sense. For example, instead of an IllegalArgumentException, it might make more sense to raise an ArithmeticException.

\$\endgroup\$
  • \$\begingroup\$ Good point, btw java.util.Objects throws the message "No java.util.Objects instances for you!". I think your suggestion is much better than what java.util.Objects throws, because this message may be presented to simple users who may take ... you! personal. \$\endgroup\$ – Peter Rader Sep 8 '15 at 11:20
1
\$\begingroup\$

Two minor points:

  1. It's confusing that you keep the same name for areEqual(double...) and areEqual(double, double). It's not clear which method is actually called if you pass two doubles. I guess it is the former, but I would have to check in the Java specification. It's probably simpler if you just name the methods differently.
  2. It's not clear that you should keep everything as static methods. You do change the state of that "object" sometimes, so why not make it an actual object. And it's possible (but unlikely) that many equality checkers could be active at the same with different threshold.
\$\endgroup\$
  • \$\begingroup\$ The former method is public. The latter is private. Does that make sense? Also, note that I have tagged it as a library. I.e. it cannot be instantiated, so there is no point in creating an instance. \$\endgroup\$ – Hungry Blue Dev Sep 7 '15 at 15:05
  • \$\begingroup\$ I know they are different methods. It is still confusing that they have the same name. And as I said, it's not even clear without reading the Java language specification which method would be called for areEqual(1.0, 1.0). \$\endgroup\$ – toto2 Sep 7 '15 at 15:08
  • 1
    \$\begingroup\$ I have never heard of a "library" in Java. I know you can't instantiate it as you wrote it, but my point was that maybe it should be a normal object which you can instantiate. \$\endgroup\$ – toto2 Sep 7 '15 at 15:10
  • \$\begingroup\$ Well java.lang.Math is a library. Try instantiating it. Also, a public method will always be called from outside the class. The only method that accesses the private method is the public one. \$\endgroup\$ – Hungry Blue Dev Sep 7 '15 at 15:13
  • 1
    \$\begingroup\$ That's not a "library". That's a plain old class with static members and a private constructor. \$\endgroup\$ – Jeroen Vannevel Sep 7 '15 at 18:34

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.