7
\$\begingroup\$

After reading Joshua Blosh's Effective Java 2nd edition, I decided to implement equals, hashcode and comparable(where applicable) to every class I have been implementing.

I have written a generic unit test class for equals, hashcode and comparable that will test every contract of each function.

I've attached the generic tester class for anyone who would be interested in seeing them.

My question is, am I overkilling the testing? Have I missed any clause in the equals, hashcode and compareTo contract? These is a tester class I've written once, and used numerous times. However, working on my own, I have not had the chance to get a second opinion with fellow developers.

My class EqualsContractTester

public class EqualsContractTester
{
    /**
     * according to Object.equals
     *for any non-null reference value x, x.equals(x) should return true.
     * @param o
     */
    public static void testReflexive(Object o)
    {
        assertEquals("Object o should be reflexibly equal to itself.", o, o);
    }
    /**
     * for any non-null reference values x and y, x.equals(y) should return true if and only if y.equals(x) returns true.
     * @param o1
     * @param o2
     */
    public static void testSymmetric(Object o1, Object o2)
    {
        assertEquals("o1 and o2 should be symetrically equal to each other.", o1, o2);

        assertEquals("o2 and o1 should be symetrically equal to each other.",o2, o1);
    }
    /**
     * for any non-null reference values x, y, and z, if x.equals(y) returns true and y.equals(z) returns true, then x.equals(z) should return true.
     * @param o1
     * @param o2
     * @param o3
     */
    public static void testTransitive(Object o1, Object o2, Object o3)
    {
        assertEquals("o1 should transitively be equal to o2.", o1, o2);

        assertEquals("o2 should transitively be equal to o3.", o2, o3);

        assertEquals("o1 should transitively be equal to o3.", o1, o3);
    }
    /**
     * For any non-null reference value x, x.equals(null) should return false.
     * @param o1
     */
    public static void testNonNullity(Object o1)
    {
        assertFalse("o1 should not be equals to null!", o1.equals(null));
    }
    /**
     * test that o1 and o2 are different
     * @param o1
     * @param o2
     */
    public static void testDifferent(Object o1, Object o2)
    {
        assertFalse("o1 should not be equals to o2.", o1.equals(o2));
    }
}

My class HashcodeContractTester

public class HashcodeContractTester
{
    /**
     * Whenever it is invoked on the same object more than once during an execution of a Java application, the hashCode method must consistently return the same integer
     */
    public static void testHashCodeConsistency(Object o1)
    {
        assertEquals("object hashcode consistency with itself failed! Weird. ", o1.hashCode(), o1.hashCode());
    }
    /**
     * if o1 is equals to o2, then o1.hashcode should be equals to o2.hashcode
     * @param o1
     * @param o2
     */
    public static void testHashCodeEquality(Object o1, Object o2)
    {
        if (o1.equals(o2))
        {
            assertEquals("if o1 and o2 are equals, then they should have the same hashcode!", o1.hashCode(), o2.hashCode());
        }
    }
}

My class ComparableTester

public class ComparableTester
{
    /**
     * ensure sgn(x.compareTo(y)) == -sgn(y.compareTo(x)) for all x and y.
     * (This implies that x.compareTo(y) must throw an exception iff y.compareTo(x) throws an exception.) </li>
     * @param <T>
     *
     * @param o1
     * @param o2
     */
    public static <T extends Comparable<T>> void assertComparisonReversal(T o1, T o2)
    {
        assertTrue("Comparison reversal should apply: sgn(o1.compareTo(o2)) == -sgn(o2.compareTo(o1)). ", signum(o1.compareTo( o2)) == -signum(o2.compareTo((T) o1)) );
    }
    /**
     * comparator should be consistent with equals if and only if e1.compareTo(e2) == 0 has the same boolean value as
     * e1.equals(e2) for every e1 and e2 of class C
     * @param o1
     * @param o2
     */
    public static <T extends Comparable<T>> void assertConsistencyWithEqual(T o1, T o2)
    {
        assertEquals("o1 and o2 should be equal. Before testing comparison. ",o1, o2);

        assertTrue("since o1 and o2 are equals, o1.compareTo(o2) should return zero!", o1.compareTo(o2) == 0);
    }
    /**
     * e.compareTo(null) should throw a NullPointerException
     * @param o1
     */

    public static <T extends Comparable<T>> void assertNullPointerException(T o1)
    {
        try
        {
            o1.compareTo(null);
            fail();
        }
        catch(NullPointerException e)
        {

        }
    }

    /**
     * (o3.compareTo(o2)>0 && o2.compareTo(o1)>0) implies o3.compareTo(o1)>0.
     *
     * @param o1
     * @param o2
     * @param o3
     */
    public static <T extends Comparable<T>> void assertTransitivity(T o1, T o2, T o3)
    {
        assertTrue("("+o3+".compareTo("+o2+") > 0) && ("+o2+".compareTo("+o1+") > 0 ) && ( "+o3+".compareTo("+o1+") > 0 )", (o3.compareTo(o2) > 0) && (o2.compareTo(o1) > 0 ) && ( o3.compareTo(o1) > 0 ));
    }
    /**
     * ensure that twinO1.compareTo(twinO2)==0 implies that sgn(twinO1.compareTo(differentO3)) == sgn(twinO2.compareTo(differentO3)), for all z.
     * @param twinO1
     * @param twinO2
     * @param differentO3
     */
    public static <T extends Comparable<T>> void assertConsistency(T twinO1, T twinO2, T differentO3)
    {
        assertTrue( (twinO1.compareTo(twinO2)==0) && (signum(twinO1.compareTo(differentO3)) == signum(twinO2.compareTo(differentO3))) );
    }
}
\$\endgroup\$
  • 1
    \$\begingroup\$ "question is, am I overkilling the testing?" <--- not possible ;-) \$\endgroup\$ – rolfl May 26 '16 at 12:21
3
\$\begingroup\$

This is a nice idea and those classes can indeed be reused on many occasions, unless equals and hashCode are generated with utils like lombok.

Here are some suggestions of how this code can be improved.

General Design

Currently, these classes contain lots of public static methods, which means that calls like ComparableTester.assertConsistencyWithEqual(o, o) look rather ugly and are too verbose in usage.

But these entities can become more flexible and easily pluggable if transformed into abstract classes or interfaces. For example:

public interface EqualsContractTester {

  public default void assertEqualsReflexive(Object o) {
    // impl...
  }

}

And if you add an abstact method like

@Test
public void testEqualsReflexive();

this already begins to look like a testing framework. Now, each test class that plugs the interface will have to implement the test method.

Moreover, using generic types (e.g. EqualsContractTester<T>) will allow to avoid to have Object args in methods, but this is optional.

Naming and Style

Method Names

The methods prefixed with test* should be renamed to assert* because assertions are what they actually do. The test* prefix is often used for test cases annotated with @Test.

Braces

Opening braces on new lines for code blocks are not Java-like style, so format them properly :)

Other

Expected Exceptions

In assertNullPointerException, instead of using a try-catch block, it is more concise to use @Test(expected = NullPointerException.class) on the test method that invokes it. The fail() call will neither be necessary in this case.

Generics

It's probably a matter of taste, but a method signature

public static <T> void assertComparisonReversal(Comparable<T> o1, Comparable<T> o2);

looks more readable for me than

public static <T extends Comparable<T>> void assertComparisonReversal(T o1, T o2)
\$\endgroup\$
  • \$\begingroup\$ Thank you! really! Your suggestions are so simple yet massively helpful! I agree with every point you have made and I shall implement them straightaway. Regards, \$\endgroup\$ – Lucas T May 27 '16 at 13:36
3
\$\begingroup\$

The following statements are made on the assumption you want to follow object-orientation.

I definitely do not suggest implementing hashcode or equals on every class. You have to look at the purpose you are following.

There are only two types of objects that are meant to use equals and hashcode that were overriden:

  1. Value objects (immutables)
  2. Business objects

Value objects

Value objects are creating their identity through ALL values they own. If they differ in only one value they can be considered as different. Value object should also be immutable.

Business objects

Business objects do have an identifier and a type to recognize in equals. In hashcode you should at least recognize the identifier. The type is optional. The identifier is used to identify the object uniquely. The type preserves the knowledge of the object's behavior.

Business objects preserve identity and behaviour but allow little mutability that is encapsulated (object orientation).

You may ask if two business objects are equal even if they differ in other attributes than identifier and type? Yes they are. Think about yourself: Even if YOU change your name, you remain the same as before. Your identity is different from your attributes.

General

Both object types have one thing in common: They are globally unique independent of the system border and they preserve their identity under every circumstances (the identity is immutable). And that is one purpose to override equals and hashcode.

These two object types also related to each other: business objects communicate with each other through value objects (messages). Here it doesn't matter if business objects communicate system internally or beyond the system border. Thats the second and the last purpose.

Other objects

If you have other objects that should not exit the system border (the JVM) then there is no necessity to override equals and hashcode. And no neccessity means you shouldn't do it.

Furthermore you can consider it as a broken design if you need to implement equals and hashcode within the system borders as the JVM provides object identity from scratch. That would mean that you may instantiate the "same" object at independent locations in your source code. That at least violates the single responsibility principle. So if you think about implementing hashcode and equals on other objects than business objects or value objects you are nearly sure to be on the wrong path. You DEFINITELY exit the path of object orientation.

You can do so but you will end up with a different programming paradigm (functional programming, only values and functions) in the best case. If you do this not strictly then you get an hybrid that will own the advantages of both paradigms but the disadvantages as well. The problem is that the assertions of on paradigm are the harm of the other paradigm and vice versa. You inherit missing encapsulation from functional programming and you inherit mutability from object orientation. So effectively you only get disadvantages.

So you see: Your decision has deep consequences if you do not follow it consequently. If you implement equals on every object, the attributes on which these methods base on mustn't change as hash-related collections are working with it (HashSet, HashMap). If they change after adding them to such a collection the objects are not predictable accesssable anymore. If you implement hashcode and equals on all objects you you win immutability on all objects and functional programming in the whole.

Conclusion

Be carefull with this decision. Think about what kind of programming paradigm you want to follow. At least for ONE module you should decide between OO or functional programming as one paradigm will eliminate the advantages of the other paradigm.

\$\endgroup\$
  • \$\begingroup\$ Thanks, @oopexpert. That is a very compelling argument and something I have not considered before. I shall pay careful consideration before implementing hashcode and equals to class objects in the future. \$\endgroup\$ – Lucas T May 27 '16 at 13:52

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.