This is a follow up post for this question.
There aren't a lot of changes. The changes (all of them major) are:
- 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. - The flexibility of the class is still kept in mind; getters and setters have been added to compliment the
private static
tolerance values. - 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
* ∞ ≠ ∞ and {@code NaN} ≠ {@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.
3 Answers 3
- Please make the class final to let noone inherit the class.
- Nobody would expect a
AsserationError
if he can inherit the class. - Everyone who inherit the class would have duplicate javadoc.
- Nobody would expect a
- Please throw a
RuntimeException
or anIllegalStateException
because the javadoc matches better. ;P
let me miss seriousness.- i miss
strictfp
at the class definition because the math-processor may wrong. getFloatTolerance
you dont need the getter. If it would be a bean, you need it but it is no bean.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).- The classname
Numbers
should be named "NumberHelper". 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?- Some methods needs to have javadoc.
I have a few more but ill stop here because this class is not thread-save.
-
\$\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 thinkNumbers.areEqual(...)
is preferable toNumberHelper.areEqual(...)
. I originally tested using JUnit4. I modified the code and presented it here. I don't thinkprivate
methods need JavaDoc if they are readable enough. You should probably take a look at the previous post. \$\endgroup\$Hungry Blue Dev– Hungry Blue Dev2015年09月07日 15:08:17 +00:00Commented Sep 7, 2015 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 yourNumbers
) came from Oracle or Sun. But StringUtils (who use similar naming like myNumberUtils
came from ASF+Friends who are much more near to the Projects than the JDK ever will be. Red or Blue pill neo. \$\endgroup\$Grim– Grim2015年09月08日 07:23:26 +00:00Commented Sep 8, 2015 at 7:23
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
.
-
\$\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 whatjava.util.Objects
throws, because this message may be presented to simple users who may take... you!
personal. \$\endgroup\$Grim– Grim2015年09月08日 11:20:27 +00:00Commented Sep 8, 2015 at 11:20
Two minor points:
- It's confusing that you keep the same name for
areEqual(double...)
andareEqual(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. - 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.
-
\$\begingroup\$ The former method is
public
. The latter isprivate
. 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– Hungry Blue Dev2015年09月07日 15:05:44 +00:00Commented Sep 7, 2015 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– toto22015年09月07日 15:08:57 +00:00Commented Sep 7, 2015 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– toto22015年09月07日 15:10:58 +00:00Commented Sep 7, 2015 at 15:10
-
\$\begingroup\$ Well
java.lang.Math
is a library. Try instantiating it. Also, apublic
method will always be called from outside the class. The only method that accesses theprivate
method is thepublic
one. \$\endgroup\$Hungry Blue Dev– Hungry Blue Dev2015年09月07日 15:13:00 +00:00Commented Sep 7, 2015 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– Jeroen Vannevel2015年09月07日 18:34:12 +00:00Commented Sep 7, 2015 at 18:34
areEqual
should beareNearlyEqual
since, as pointed out in a response to the earlier question, this version of equality is not transitive, hence, hides a trap. \$\endgroup\$