Java equals(): to reflect or not to reflect

StackOverflow https://stackoverflow.com/questions/124585

  •  02-07-2019
  •  | 
  •  

Question

This question is specifically related to overriding the equals() method for objects with a large number of fields. First off, let me say that this large object cannot be broken down into multiple components without violating OO principles, so telling me "no class should have more than x fields" won't help.

Moving on, the problem came to fruition when I forgot to check one of the fields for equality. Therefore, my equals method was incorrect. Then I thought to use reflection:

--code removed because it was too distracting--

The purpose of this post isn't necessarily to refactor the code (this isn't even the code I am using), but instead to get input on whether or not this is a good idea.

Pros:

  • If a new field is added, it is automatically included
  • The method is much more terse than 30 if statements

Cons:

  • If a new field is added, it is automatically included, sometimes this is undesirable
  • Performance: This has to be slower, I don't feel the need to break out a profiler
  • Whitelisting certain fields to ignore in the comparison is a little ugly

Any thoughts?

Was it helpful?

Solution

If you did want to whitelist for performance reasons, consider using an annotation to indicate which fields to compare. Also, this implementation won't work if your fields don't have good implementations for equals().

P.S. If you go this route for equals(), don't forget to do something similar for hashCode().

P.P.S. I trust you already considered HashCodeBuilder and EqualsBuilder.

OTHER TIPS

Use Eclipse, FFS!

Delete the hashCode and equals methods you have.

Right click on the file.

Select Source->Generate hashcode and equals...

Done! No more worries about reflection.

Repeat for each field added, you just use the outline view to delete your two methods, and then let Eclipse autogenerate them.

If you do go the reflection approach, EqualsBuilder is still your friend:

 public boolean equals(Object obj) {
    return EqualsBuilder.reflectionEquals(this, obj);
 }

Here's a thought if you're worried about:

1/ Forgetting to update your big series of if-statements for checking equality when you add/remove a field.

2/ The performance of doing this in the equals() method.

Try the following:

a/ Revert back to using the long sequence of if-statements in your equals() method.

b/ Have a single function which contains a list of the fields (in a String array) and which will check that list against reality (i.e., the reflected fields). It will throw an exception if they don't match.

c/ In your constructor for this object, have a synchronized run-once call to this function (similar to a singleton pattern). In other words, if this is the first object constructed by this class, call the checking function described in (b) above.

The exception will make it immediately obvious when you run your program if you haven't updated your if-statements to match the reflected fields; then you fix the if-statements and update the field list from (b) above.

Subsequent construction of objects will not do this check and your equals() method will run at it's maximum possible speed.

Try as I might, I haven't been able to find any real problems with this approach (greater minds may exist on StackOverflow) - there's an extra condition check on each object construction for the run-once behaviour but that seems fairly minor.

If you try hard enough, you could still get your if-statements out of step with your field-list and reflected fields but the exception will ensure your field list matches the reflected fields and you just make sure you update the if-statements and field list at the same time.

You can always annotate the fields you do/do not want in your equals method, that should be a straightforward and simple change to it.

Performance is obviously related to how often the object is actually compared, but a lot of frameworks use hash maps, so your equals may be being used more than you think.

Also, speaking of hash maps, you have the same issue with the hashCode method.

Finally, do you really need to compare all of the fields for equality?

You have a few bugs in your code.

  1. You cannot assume that this and obj are the same class. Indeed, it's explicitly allowed for obj to be any other class. You could start with if ( ! obj instanceof myClass ) return false; however this is still not correct because obj could be a subclass of this with additional fields that might matter.
  2. You have to support null values for obj with a simple if ( obj == null ) return false;
  3. You can't treat null and empty string as equal. Instead treat null specially. Simplest way here is to start by comparing Field.get(obj) == Field.get(this). If they are both equal or both happen to point to the same object, this is fast. (Note: This is also an optimization, which you need since this is a slow routine.) If this fails, you can use the fast if ( Field.get(obj) == null || Field.get(this) == null ) return false; to handle cases where exactly one is null. Finally you can use the usual equals().
  4. You're not using foundMismatch

I agree with Hank that [HashCodeBuilder][1] and [EqualsBuilder][2] is a better way to go. It's easy to maintain, not a lot of boilerplate code, and you avoid all these issues.

You could use Annotations to exclude fields from the check

e.g.

@IgnoreEquals
String fieldThatShouldNotBeCompared;

And then of course you check the presence of the annotation in your generic equals method.

If you have access to the names of the fields, why don't you make it a standard that fields you don't want to include always start with "local" or "nochk" or something like that.

Then you blacklist all fields that begin with this (code is not so ugly then).

I don't doubt it's a little slower. You need to decide whether you want to swap ease-of-updates against execution speed.

Licensed under: CC-BY-SA with attribution
Not affiliated with StackOverflow
scroll top