Is it good practice to use domain objects as keys?

Question

Is is good practice to use domain objects as keys for maps (or "get" methods), or is it better to just use the id of the domain object?

It's simpler to explain with an example. Let's say I have Person class, a Club class, and a Membership class (that connects the other two). I.e.,

public class Person {
    private int id; // primary key
    private String name;
}

public class Club {
    private String name; // primary key
}

public class Membership {
    private Person person;
    private Club club;
    private Date expires;
}

Or something like that. Now, I want to add a method getMembership to Club. The question is, should this method take a Person object:

public Membership getMembership(Person person);

or, the id of a person:

public Membership getMembership(int personId);

Which is most idiomatic, which is most convenient, which is most suitable?

Edit: Many very good answers. I went with not exposing the id, because the "Person" (as you might have realized, my real domain does not have anything to do with people and clubs...) instances are easily available, but for now it is internally stored in a HashMap hashed on the id - but at least I am exposing it correctly in the interface.

Answer 1

Assuming this is a database ID or something used just for indexing (rather than something like an SSN), then in an ideal system, the presence of an ID is an implementation detail.

As an implementation detail, I would prefer to hide it in the interface of other domain objects. Thus, membership involves, fundamentally, individuals rather than numbers.

Of course, I'd make sure I implemented hashCode and equals() and documented well what they meant.

In that case, I would explicitly document that the equality of two Person objects is determined solely based on ID. This is somewhat a risky proposition, but makes code more readable if you can ensure it. I feel more comfortable making it when my objects are immutable, so I would not actually end up with two Person objects with the same ID but different names in the lifetime of the program.

Answer 2

I think the first case would be considered "purer" in the sense that the getMembership method might require more specific data from the person itself other than its id (Let's assume you do not know the internals of the getMembership method, even though this makes little sense since it's most likely in the same domain).

If it turns out that it actually requires data from the Person entity then it will not require a DAO or factory for the person in question.

This can be easily called if your language and/or ORM allows you to use proxy objects (and if you have a convenient way to create these proxies).

But lets be honest. If you're inquiring about some membership of a person, you most likely already have this Person instance in memory at hand when you call this method.

Further down the road in the "infrastructure land" there's also this notion about implementation details which Uri already mentioned while I was writing this answer (damn, that was fast bro'!). To be specific, what if you decided that this 'Person' concept suddenly has a composite primary key/identifier in the underlying database... Would you now use an identifier class? Perhaps use that proxy we were talking about?

TL;DR version

Using ID's is really easier in the short run, but if you're already using a solid ORM, I see no reason not to use proxies or some other means to express the object oriented identity of an Entity which doesn't leak implementation details.

Answer 3

I would typically stick with less is more. The less information required to invoke your method the better. If you know the ID, only require the ID.

If you want, provide extra overloads which accept extra parameters, such as the entire class.

Answer 4

IMO, I think it very much depends on the flow of the application - do you have the Person available when you want to get the Membership details? If yes, go with:
public Membership getMembership(Person person);

Also, I don't see any reason why the Club cannot keep track of memberships based on the Person's ID and not the actual object - I think that would mean you don't need to implement the hashCode() and equals() methods. (Although that is always a good best-practice).

As Uri said, you should document the deceleration that two Person objects are equal if their ID is equal.

Answer 5

If you already have the object, there's no reason to pull out the ID to get a hash key.

As long as the IDs are always unique, implement hashCode() to return the ID, and implement equals() as well.

Odds are every time you'll need the Membership, you'll already have the Person, so it saves code and confusion later.

Answer 6

Whoa. Back up a sec here. The getMembership() method doesn't belong in Club. It belongs to the set of all memberships, which you haven't implemented.

Answer 7

First of all I'd put any getters of such nature inside a DAO (and not on the model). Then I'd use the entity itself as a parameter, and what happens inside the method is an implementation detail.

Answer 8

If you are really practicing object oriented design, then you want to invoke the idea of information hiding. As soon as you start hanging internal field types of the person object in the public interface of the membership object's methods, you start forcing external developers (users) of your objects to start learning all kinds of information about what a person object is, and how it is stored, and what kind of ID it has.

Better yet, since a person can have memberships, why don't you just hang the "getMemberships" method onto the person class. It seems much more logical to ask a person which memberships they have, than to ask a "membership" which clubs a given person may belong to...

Edit - since the OP has updated to indicate that it is the membership itself that he is interested in, and not just used as a relation between Person and Club, I'm updating my answer.

Long story short, the "Club" class that you are defining, you are now asking to behave as a "club roster". A club has a roster, it isn't is a roster. A roster could have several features, including ways to look up persons belonging to the club. In addition to looking up a person by their club ID, you might want to look them up by SSN, name, join date, etc.. To me, this says there is a method on class "Club" called getRoster(), which returns a data structure that can lookup all the persons in the club. Call it a collection. The question then becomes, can you use the methods on pre-existing collections classes to fulfill the needs you have defined so far, or do you need to create a custom collection subclass to provide the appropriate method to find the membership record.

Since your class heirarchy is most likely backed by a database, and you are probably taking about loading info out of the database, and don't necessarily want to get the entire collection just to get one membership, you may want to create a new class. This class could be called as I said "Roster". You would get the instance of it from the getRoster() call on class "club". You would add "searching" methods to the class based on any search criteria you wanted that was "publicly available" information about the person.. name, clubID, personID, SSN, etc...

My original answer only applies if the "membership" is purely a relation to indicate which clubs which persons belong to.

Answer 9

Unless there's a significant benefit derived elsewhere, it can be said that keys in map should single-valued things, if at all possible. That said, through paying attention to equals() and hashCode() you can make any object work as key, but equals() and hashCode() aren't very pleasing things to have to pay attention to. You'll be happier sticking to IDs as keys.

Answer 10

Don't use the id's man, this is just a bad idea for all the reasons mentioned. You'll lock yourself into a design. Let me give an example.

Right now you define you're Membership as a mapping between Clubs to People. Rightfully, your Membership should be a map of Clubs to "Members", but you are assuming that all Members are People and that since all of the people id's are unique you think you can just use the ID.

But what if in the future you want to extend your membership concept to "family memberships", for which you create a Family table and a Family class. In good OO fashion you extract an interface of Family and Person called Member. As long as both classes implement the equals and hashCode methods properly, no other code will have to be touched. Personally, I would have defined the Member interface right up front.

public interface Member {
}

public class Person implements Member {
    private int id; // primary key
    private String name;
}

public class Family implements Member {
   private int id;
   private String name;
}

public class Club {
    private String name; // primary key
}

public class Membership {
   private Member member;
   private Club club;
   private Date expires;
}

If, you had used ID's in your interface, you will either need to enforce cross-table uniqueness of key values, or maintain two separate Maps and forgo the nice polymorphic interface stuff.

Believe me, unless you are writing one-off, disposable applications, you want to avoid using ID's in your interface.

Answer 11

I would probably use IDs. Why? By taking IDs, I'm making safer assumptions about the caller.

If I have an ID, how much work is it to get the Person? Might be 'easy', but it does require hitting a datastore, which is slow...

If I have Person object, how much work is it to get the ID? Simple member access. Fast and available.

Answer 12

Actually, what I would do is call it by id, but refactoring a bit the original design:

public class Person {
    private int id; // primary key
    private String name;
}

public class Club {
    private String name; // primary key
    private Collection<Membership> memberships;
    public Membership getMembershipByPersonId(int id);
}

public class Membership {
    private Date expires;
    private Person person;
}

or

public class Person {
    private int id; // primary key
    private String name;
    private Membership membership;
    public Membership getMembership();
}

public class Club {
    private String name; // primary key
    private Collection<Person> persons;
    public Person getPersonById(int id);
}

public class Membership {
    private Date expires;
}

Answer 13

As described by others: use the object.

I work on a system where we had some old code that used int to represent transaction ids. Guess what? We started running out of ids because we used int.

Changing to long or BigNumber proved tricky because people had become very inventive with naming. Some used

int tranNum

some used

int transactionNumber

some used

int trannNum

(complete with spelling mistakes).

Some people got really inventive...

It was a mess and sorting it out was a nightmare. I ended up gping through all of the code manually and converting to a TransactionNumber object.

Hide the details wherever possible.