Win a copy of Five Lines of Code this week in the OO, Patterns, UML and Refactoring forum!
  • Post Reply Bookmark Topic Watch Topic
  • New Topic
programming forums Java Mobile Certification Databases Caching Books Engineering Micro Controllers OS Languages Paradigms IDEs Build Tools Frameworks Application Servers Open Source This Site Careers Other all forums
this forum made possible by our volunteer staff, including ...
Marshals:
  • Campbell Ritchie
  • Bear Bibeault
  • Ron McLeod
  • Jeanne Boyarsky
  • Paul Clapham
Sheriffs:
  • Tim Cooke
  • Liutauras Vilda
  • Junilu Lacar
Saloon Keepers:
  • Tim Moores
  • Stephan van Hulst
  • Tim Holloway
  • fred rosenberger
  • salvin francis
Bartenders:
  • Piet Souris
  • Frits Walraven
  • Carey Brown

Number of tests needed for equals and hashCode contract

 
Ranch Hand
Posts: 49
1
Netbeans IDE Java Linux
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
I have been using EqualsVerifier and I have attempted to write tests myself but I never felt comfortable writing them since finding EqualsVerifier. I have been thinking lately that there must be a formula for how many tests are needed to ensure the equals and hashCode contract is met. Say there is a typical pojo.



Given that there are only two member variables and that an IDE or lombok generates equals and hashCode how many tests would be needed to test these methods?
 
Sheriff
Posts: 21972
106
Eclipse IDE Spring VI Editor Chrome Java Ubuntu Windows
  • Likes 1
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
If Lombok generates your equals and hashCode methods I personally wouldn't spend time testing it. They know what you're doing, and you'd be testing Lombok more than your own code.
If your IDE generates your equals and hashCode methods you need to test it thoroughly. Sure, what it has generated now will work just fine, but if you add a field and forget to update equals and/or hashCode, your method(s) may be broken.

I like to take control, so I write my own (parameterized) equals tests. What I basically do is create one instance, then compare it with:
* itself
* null
* a different type (usually a String)
* for each field, an object of the same type with just that field set differently

Because I always use Objects.equals for nullable fields I don't bother checking with null and non-null separately. If you'd do that, you get 4 tests for each field for the null / non-null combinations (although you can combine a few).

As for hashCode, I don't really test that as well. I do test that the object's hashCode is the same when called twice, and that it's the same as an equal object. If fields are not used in hashCode you'd need to test with objects with different values for these fields as well.
 
Saloon Keeper
Posts: 12133
258
  • Likes 1
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
I've once written a test framework that included the following tests:

  • equals(null) is always false.
  • equals() is reflexive.
  • equals() is symmetric.
  • equals() is transitive.
  • hashCode() for equal objects returns equal results.

  • These tests were very easily written in JUnit 4 using the experimental Theories, and they tested a wide range of objects. I think in JUnit 5 you can maybe do the same thing with parameterized tests.

    Sadly, I don't know where I left the code, but one of the tests might have looked like this:

    A test class might then look something like this:
     
    Marshal
    Posts: 69779
    277
    • Likes 1
    • Mark post as helpful
    • send pies
    • Quote
    • Report post to moderator

    Rob Spoor wrote:. . . if you add a field . . .

    If you simply add a field, equals() and hashCode() will still fulfil their general contract, even though you will now have a peculiar definition of equality.See line 5.You can now have two cars with the same make and number but travelling at different speeds and neither equals() nor hashCode() will notice a difference. Strange, but it is a conceivable situation in real life, particularly amongst the criminal classes Your troubles will start if you update equals() or hashCode() but not both; I think updating only hashCode() will break equals() but not vice versa.
    Another way to break those methods with an additional field is to add that field in a subclass.Round here, it is legal for taxis to be registered in several boroughs, so the charge can differ on where the fare is picked up. If you use the charge in Taxi#hashCode() only, goodbye general contract for hashCode(). And if you use it in equals(), you have the same problem as Point/ColoredPoint (Joshua Bloch: described in Effective Java, and I think Angelika Langer uses the same example). Goodbye transitivity, and probably goodbye symmetry. Whether a tool will generate such a method, I don't know.
    I am sure Rob is correct that generated code will be correct and doesn't need testing. But if such a method is generated in subtypes, then the generated code can be incorrect. If your tool generates an equals() method in Car and in Taxi, even if you remove chargePerMile from the method, you will still have incorrect code because one version will include ...ob instanceof Car... and the other ...ob instanceof Taxi... Goodbye symmetry. Some of the errors you get in handwritten code only can be avoided with @Override. But do any of your tests compare hash codes?
     
    John Mercier
    Ranch Hand
    Posts: 49
    1
    Netbeans IDE Java Linux
    • Mark post as helpful
    • send pies
    • Quote
    • Report post to moderator
    I’m starting to see how this question depends on a few factors. I think these are the easy tests.

    Self
    Null
    Reflexive
    Symmetric
    Transitive

    So that is 5 tests at least. Then for each member variable being checked there needs to be two tests that shows a failure when the member variables are not equal. So maybe the equation is 5+2m. I picked 2m because for each member there is a test for the left side being null and the right side being null.

    I believe that most of the time when a field is added equals and hashCode should be considered for an update and this is a common mistake but let's just assume all fields are considered.

    The use of Objects.equals helps in preventing NullPointerExceptions. Deciding not to add null tests could be coupling the tests to the implementation of equals. To avoid this, I think in most cases they should be added. One exception is if there is no way to set a member to null through the api. I’m assuming a typical pojo with default constructor and setter for every member. So I’m adding 2n for nullable member variables. So far I have:

    5 + 2m + 2n

    m is the number of member variables being compared

    n is the number of nullable member variables being compared

    So far this only seems to cover equals but not hashCode. Is there really a way to test hashCode without knowing the implementation. If hashCode can always return 1 and still be valid I think it would need to be tested in a way that shows it is a good hashCode. It should be unique for everything that is equal and different for things that are not equal. If this is true then each of the above tests could check hashCode as well.
     
    Stephan van Hulst
    Saloon Keeper
    Posts: 12133
    258
    • Likes 1
    • Mark post as helpful
    • send pies
    • Quote
    • Report post to moderator

    John Mercier wrote:If hashCode can always return 1 and still be valid I think it would need to be tested in a way that shows it is a good hashCode.


    This is very difficult. How good a hashCode() implementation is is determined by how uniformly objects are distributed over the set of ints. You would need to perform tests on lots of objects to get a sample distribution and then perform statistical analysis to determine if the distribution is uniform enough, for some predetermined value "enough".

    It should be unique for everything that is equal


    You mean equal for everything that are equal.

    and different for things that are not equal


    Not true. You can generate an infinite amount of distinct objects from most classes. Case in point: I can generate an infinite amount of unique strings to use for the source field of your DirectedEdge class. How are you going to map an infinite amount of possible DirectedEdge instances to ints so that each one uses a unique int value as its hash code?

    The only test you can perform for hashCode() is that equal objects must have equal hash codes.
     
    Campbell Ritchie
    Marshal
    Posts: 69779
    277
    • Mark post as helpful
    • send pies
    • Quote
    • Report post to moderator

    John Mercier wrote:. . . I believe that most of the time when a field is added equals and hashCode should be considered for an update and this is a common mistake but let's just assume all fields are considered.

    Please explain more. That sentence is rather vague. It is not always necessary to use every field in equals() but you must only use fields in hashCode() which are used in equals(). If you use a feature to calculate a hash code which isn't used to determine equality, you can have two objects equal to each other but with different hash code.

    The use of Objects.equals helps in preventing NullPointerExceptions.

    No. Most people don't write super.equals(ob) in their equals() method, but many people write an == test, which does exactly the same.Both those partial methods will do the same, but neither will cure any problems caused by passing null as an argument. If you look at the general contract for equals(), you find it must accept null and always return false. If you look at the point about consistency, you find it should always return a result, which means no exceptions are permitted. Both the following partial methods will satisfy that requirement, but you must remember that the version with getClass() is a bit too stringent, not permitting equality with subclass objects:-Remember that instanceof returns false if its left operand is null.
    Failing to reject nulls is failing to maintain the general contract (non‑nullity).

    . . .

    Edit: remove rest of quote. I pushed the submit button too early.
     
    Campbell Ritchie
    Marshal
    Posts: 69779
    277
    • Mark post as helpful
    • send pies
    • Quote
    • Report post to moderator

    A few minutes ago, I wrote:. . . Both those partial methods will do the same . . ..

    Actually, that is only true if your superclasses don't override equals(). It is probably a bad idea to override equals() more than once because you can get transitivity failure, and you would find that difficult to test automatically. Maybe you should always make equals() a final method. Just a suggestion.
     
    John Mercier
    Ranch Hand
    Posts: 49
    1
    Netbeans IDE Java Linux
    • Mark post as helpful
    • send pies
    • Quote
    • Report post to moderator

    Stephan van Hulst wrote:The only test you can perform for hashCode() is that equal objects must have equal hash codes.



    Ok I think that hashCode could be checked in the equals tests. When two objects are expected to be equal the hashCode must be equal as well.

    Campbell Ritchie wrote:Please explain more



    What I mean is I often see developers add fields to classes without considering equals or hashCode at all. This is something I check in code reviews. It is possible some fields are not needed in equals and hashCode but it is still something that should be considered each time a field is added. One time we had a field added and it was added to equals and hashcode but in a way that would cause an NPE if the field was null.

    Campbell Ritchie wrote:No. Most people don't write super.equals(ob) in their equals() method, but many people write an == test, which does exactly the same.



    Using Objects.equals to check member variables helps prevent NPE. This is what intellij generates using Objects.equals if null members are allowed.



    This is what it generates when not using Objects.equals



    Objects.equals helps because it already does the null checking on its parameters. I'm not sure what this has to do with inheritance at this point but I think that could be considered a +1 for the number of tests needed.

    I agree that testing with a null parameter is needed.



    This was covered in my previous post.

    So I think the number of tests needed could be

    5 + 2 x members + 2 x nullableMembers + 1 x superclass (can be 0)


    In the case of DirectedEdge if source and target are non-null then this would be 9 tests for equals.

    * self
    * null
    * reflexive
    * symmetric
    * transitive
    * sources equal
    * sources not equal
    * targets equal
    * targets not equal
     
    Campbell Ritchie
    Marshal
    Posts: 69779
    277
    • Mark post as helpful
    • send pies
    • Quote
    • Report post to moderator

    John Mercier wrote:. . . I often see developers add fields to classes without considering equals or hashCode at all. . . .

    Whether they realise it or not, those people are creating a definition of equality which ignores those fields. Just as my Car class defined equality without using speed. Of course, ti is a lot better when people understand what they are doing, rather than its happening by default.

    One time we had a field added and it was added to equals and hashcode but in a way that would cause an NPE if the field was null. . . .

    You always have to consider the possibility of a field being null when you write equals(). You can writeMany people wrote equality tests like that until the Objects class was introduced (Java7; it should have been Java1.1).

    . . . using Objects.equals . . .

    I am so sorry; I misread what you wrote. I thought you said Object#equals() and you wrote Objects#equals(). I was totally mistaken about what you meant.
     
    John Mercier
    Ranch Hand
    Posts: 49
    1
    Netbeans IDE Java Linux
    • Mark post as helpful
    • send pies
    • Quote
    • Report post to moderator

    Campbell Ritchie wrote:You always have to consider the possibility of a field being null when you write equals().



    This is true but I was just thinking that if the api does not allow the field to ever be null this check would not be needed. I think this is why in the intellij generator it asks which fields are non-null. It doesn't appear to add the null checks in those cases.



    The api can do this by doing adding required parameters and null checks in the constructor or in any method that sets the field.

    Campbell Ritchie wrote:I am so sorry



    Thanks, that is fine. I was a little confused but I think it helped bring up the point about super classes. I think I do remember these issues being discussed in Effective Java. I will have to find that part of the book soon.
     
    Stephan van Hulst
    Saloon Keeper
    Posts: 12133
    258
    • Mark post as helpful
    • send pies
    • Quote
    • Report post to moderator

    John Mercier wrote:Ok I think that hashCode could be checked in the equals tests.


    No. Tests must be simple and test one specific thing. If your hashCode() is implemented improperly, you don't want that to trigger a failed test that implies that equals() isn't implemented properly.

    What I mean is I often see developers add fields to classes without considering equals or hashCode at all.


    There's not much you can do against this, except having different people design and implement the class. The person that designs the class should also write the test. If both programmers fail at writing proper tests and implementations and nobody catches it, you have a bug. It happens.

    It is possible some fields are not needed in equals and hashCode


    It's NOT normal to have fields that are not included in the equality definition, unless the fields are only used to cache expensive calculations based on other fields. Classes are either value types in which case they have a definition of equality that is based on all normalized fields, or they are entities or services, in which case different instances are unequal.

    One time we had a field added and it was added to equals and hashcode but in a way that would cause an NPE if the field was null.


    Why was the field allowed to be null? This should occur only very rarely. For instance, I don't see the purpose of having an edge with a null source or target.

    This is what intellij generates using Objects.equals if null members are allowed.


    I don't like the code that IntelliJ generates, because it uses getClass(). That implies that you can't compare instances of base classes against instances of derived classes. Use instanceof instead and make your equals() method final.

    So I think the number of tests needed could be

    5 + 2 x members + 2 x nullableMembers + 1 x superclass (can be 0)


    Not sure what you mean by this, but in general the number of  tests you need is not a simple calculation over the number of members of a class. You don't test members. You test assumptions you make about the class. The number of assumptions depend on the class design, not the number of members.

    In the case of DirectedEdge if source and target are non-null then this would be 9 tests for equals.


    I'm not sure what you mean by the self test. Isn't it already covered by the reflexivity test?


    I started writing an example of how I would write your DirectedEdge class, and tests for it, but I got distracted. I might post it later.
     
    John Mercier
    Ranch Hand
    Posts: 49
    1
    Netbeans IDE Java Linux
    • Mark post as helpful
    • send pies
    • Quote
    • Report post to moderator

    Stephan van Hulst wrote:No. Tests must be simple and test one specific thing.



    I agree it wouldn't make sense to have an equals transitive test fail for hashCode.

    Stephan van Hulst wrote:Classes are either value types in which case they have a definition of equality that is based on all normalized fields, or they are entities or services, in which case different instances are unequal.



    Thanks for posting this. Especially the part about entities. I believe The Car example where speed is ignored by Campbell Ritchie would be and entity but it seems ok to check some of the fields in the equals method. For instance if equals checked an id field for a database entity.

    Stephan van Hulst wrote:Why was the field allowed to be null? This should occur only very rarely. For instance, I don't see the purpose of having an edge with a null source or target.



    This was in an actual work example where a field was added to an object that could have been null.

    Stephan van Hulst wrote:I don't like the code that IntelliJ generates, because it uses getClass(). That implies that you can't compare instances of base classes against instances of derived classes. Use instanceof instead and make your equals() method final.



    I think that would be fine but maybe the whole class could be final?

    Stephan van Hulst wrote:Not sure what you mean by this, but in general the number of  tests you need is not a simple calculation over the number of members of a class. You don't test members. You test assumptions you make about the class. The number of assumptions depend on the class design, not the number of members.



    I'm only trying to calculate the number of tests needed for equals and hashCode in a class. Since these methods are based entirely on the members of the class I thought the number of tests could be determined ahead of time.

    Stephan van Hulst wrote:I'm not sure what you mean by the self test. Isn't it already covered by the reflexivity test?



    Yes thanks for pointing that out.
     
    Stephan van Hulst
    Saloon Keeper
    Posts: 12133
    258
    • Likes 1
    • Mark post as helpful
    • send pies
    • Quote
    • Report post to moderator

    John Mercier wrote:I believe The Car example where speed is ignored by Campbell Ritchie would be and entity but it seems ok to check some of the fields in the equals method. For instance if equals checked an id field for a database entity.


    I don't agree. Two different objects that have the same ID but otherwise different properties are simply not equal. Many people make the mistake of making database entities equatable by their ID property only.

    If you want to see if two entities refer to the same entry in a database, then simply compare their IDs:

    I think that would be fine but maybe the whole class could be final?


    Yes. It's always a good idea to make the entire class final. I still prefer the instanceof operator in final classes because it also performs the null check.

    I'm only trying to calculate the number of tests needed for equals and hashCode in a class. Since these methods are based entirely on the members of the class I thought the number of tests could be determined ahead of time.


    Sure, but you only need to write a fixed number of tests that don't depend on the number of properties. If you add another property to your class, you only need to edit your existing tests (assuming you wrote parameterized tests to begin with).
     
    Campbell Ritchie
    Marshal
    Posts: 69779
    277
    • Mark post as helpful
    • send pies
    • Quote
    • Report post to moderator

    Stephan van Hulst wrote:. . . I still prefer the instanceof operator in final classes because it also performs the null check. . . .

    If you make the class final, then your equals() method can be correct whether you use instanceof or getClass(). Assuming we would permit equality to subtype objects, if you take the final modifier off the class, then you are allowed subtypes, and you can't put that modifier back. It also means you would have to change the equals() method if you used getClass().
    Isn't that another advantage for instanceof?
     
    John Mercier
    Ranch Hand
    Posts: 49
    1
    Netbeans IDE Java Linux
    • Mark post as helpful
    • send pies
    • Quote
    • Report post to moderator

    Stephan van Hulst wrote:assuming you wrote parameterized tests to begin with



    I see what you mean. It would be a new set of parameters for the tests. I thought each set for a parameterized test was considered a test. What you are saying makes sense to me now.

    I would have to look into the database example more. I haven't been working with database Entities lately and I'm not sure if I ever implemented equals on one.
     
    Clowns were never meant to be THAT big! We must destroy it with this tiny ad:
    Thread Boost feature
    https://coderanch.com/t/674455/Thread-Boost-feature
      Bookmark Topic Watch Topic
    • New Topic