CODEWITHSUNDEEP
javaequals
secretpow
secretpow

Reputation: 345

Should I compare all fields in my class's "equals" method?

I'm working on an application that allows the user to manage accounts. So, suppose I have an Account class, representing one of the user's accounts:

class Account
{
    public int id;
    public String accountName;
    public String accountIdentifier;
    public String server;
    public String notes;
}

My equals method looks like this:

public boolean equals(Object o)
{
    if (this == o)
        return true;
    if (o == null || !(o instanceof Account))
        return false;

    Account other = (Account) o;
    if (!accountIdentifier.equals(other.accountIdentifier))
        return false;
    if (!server.equals(other.server))
        return false;
    return true;
}

As you can see, I'm only comparing the accountIdentifier and the server, but not the other fields. There are several reasons why I chose this approach.

  1. I keep the accounts in a List. When the user updates an account, by changing the account name (which is just a name specified by the user to identify the account) or the notes, I can do accountList.set(accountList.indexOf(account), account); to update the account in the list. If equals compared all properties, this approach wouldn't work, and I'd have to work around it (for example by iterating over the list and checking for these properties manually).
  2. This might actually be more important, but it only came to my mind after thinking about it for a while. An Account is uniquely identified by the accountIdentifier and the server it belongs to. The user might decide to rename the account, or change the notes, but it's still the same account. But if the server is changed, I think I would consider it a different account. The id is just an internal ID since the accounts are stored in a database. Even if that changed, the account is still considered the same account if the accountIdentifier and the server stayed the same.

What I'm trying to say is that I basically implemented equals this way to allow for shorter, more concise code in the rest of the application. But I'm not sure if I'm breaking some rules here, or if I'm doing something that might cause other developers headaches if it ever happens that someone is working with my application's API.

Is it okay to only compare some fields in the equals method, or should I compare all fields?

Upvotes: 6

Views: 6958

Answers (5)

eddiewould
eddiewould

Reputation: 1633

The answer is "it depends depends on the semantics of your data".

For example, you might internally store a field that can be derived (calculated) from the other fields. In which case, you don't need to compare the calculated value.

As a gross generalisation, anything that cannot be derived from other fields should be included.

Upvotes: 1

Rigre Garciandía
Rigre Garciandía

Reputation: 159

For the key normally you should use the business key, this key can be simple or composite key and not necessary need to include all the fields in the entity. So... depends of each case to select what identify an entity. If possible should be the minimum number of field fully and unique identify the entity.

Some people prefer (and is a good practice) to create a surrogate key that will identity the object, this is very useful when you want to persist your objects using any ORM due you don’t need to export the keys to the child entities in 1:M or M:N relations. For example the ID in your sample can be considered as surrogate key if you create it as internal unique identifier.

Also may want to take into consideration:

  • Always you override equals you must override hashCode too, this is important to work properly with classes like Collections, Maps etc
  • Apache provide a really nice API to help in the implementation of equals and hashCode. Those classes are EqualsBuilder and HashCodeBuilder. Both allow you to concatenate the fields you want to use in your comparison and have a way also to use reflection.

Upvotes: 1

yshavit
yshavit

Reputation: 43391

Yes, it's definitely okay to do this. You get to decide what equality means for your class, and you should use it in a way that makes the most sense for your application's logic — in particular, for collections and other such classes that make use of equality. It sounds like you have thought about that and decided that the (server, identifier) pair is what uniquely distinguishes instances.

This would mean, for instance, that two instances with the same (server, identifier) pair but a different accountName are different versions of the same Account, and that the difference might need to be resolved somehow; that's a perfectly reasonable semantic.

It may make sense to define a separate boolean allFieldsEqual(Account other) method to cover the "extended" definition, depending on whether you need it (or would find it useful for testing).

And, of course, you should override hashCode to make it consistent with whatever definition of equals you go with.

Upvotes: 6

Rohan
Rohan

Reputation: 551

You should compare all of the fields that are necessary to determine equality. If the accountIdentifier and server fields are enough to determine if two objects are equal, then that is perfectly fine. No need to include any of the other fields that don't matter in terms of equality.

Upvotes: 2

Makoto
Makoto

Reputation: 106460

This is fine - and probably a good thing to do. If you've identified equality as the accountIdentifier and the server being distinct and unique, then that's perfectly valid for your use case.

You don't want to use more fields than you need to since that would produce false positives in your code. This approach is perfectly suitable to your needs.

Upvotes: 0

Related Questions