Should web app domain objects containing collections have "add" and "remove" item methods?

Question

Let's say I have a Person class that has a collection of Dog objects. The relationship is bidirectional.

public class Person {
  private List<Dog> dogs;
  // getter and setter for dogs
}

public class Dog {
  private Person person;
  // getter and setter for person
}

Ok now if I was just working with these objects, I would have methods in Person for adding a dog and removing a dog, so that the client isn't directly working with the collection.

public void removeDog(Dog dog) {
  dogs.remove(dog);
  dog.setPerson(null);
}

public void addDog(Dog dog) {
  dog.setPerson(this);
  dogs.add(dog);
}

or something along those lines.

My question comes when working with these objects in the service layer. I can add a new dog to a person via addDog and simply save the person which will automatically persist the new dog. However, removing a dog via removeDog and then saving the person will not automatically delete the dog from persistent storage...it'll just set the dog reference to null.

So I end up with something like:

Dog dog = dogDAO.getDogById(int id);
dogDAO.remove(dog);

Because this is a web app and Persons and Dogs are not maintained in memory, this works fine, and I never called Person.removeDog(). The next time I load up the Person who originally referenced this dog, it will not contain the dog because it was removed from storage. The same thing can apply to adding a dog:

Person person = personDAO.getPersonById(int id);
Dog dog = new Dog();
dog.setPerson(person);
dogDAO.save(dog);

Again this works fine even and I never called Person.addDog().

Is there any point to having the addDog and removeDog methods in my Person class then?

Answer 1

This is always hard to do without manual intervention. Once you remove the object reference and have a stale object. In the other use case you just remove the object to which some other object points (the list). This should be a problem, too. If this is accepted to lazily it will cover a lot of errors.

For simplicity you could add the dao remove operation to the person removeDog() but then the use case removing the dog from one person and add it to another wouldn't be possible. What is the problem that you remove the dog from the person and then remove it from the persistence if you know it has to be removed at all? You spare one step. So returning the removed dog will bring you a pattern of

dogDAO.remove(person.removeDog(dog))

Having the add and remove in person enables you not only to mimick the list behaviour but also to have some extra functionality. In your case it is hard to decide because it is an easy setup and it depends how accesible the dao stuff is and where you like to put that.

You should at least have an impression of every single use case you want to support. Then the mandatory and optional object links and settings need to be defined. With some pragmatism (e.g. coupling the persistence stuff with the objects) we have to chance to do some more object oriented setup that is easy (and therefor less error prone) to use

Answer 2

If, logically, adding and removing dogs from people is how it will be done, then there is no reason to have methods for adding and removing people from dogs. Your goal is that your data model is always in a consistent state. If you allow removing people from dogs, then you'll have to have the dog remove itself from the person to keep your data model consistent. Then you'll have to avoid recursion.

It sounds like the way you have it is fine and clear and unambiguous. If you want, you can always add a comment to the Dog class to add that it's purposeful that there's no addPerson and removePerson method, and that this should be done instead by removing the dog from the person or adding the dog to the person.

There is no need for perfect symmetry or perfect orthogonality in code. The goal is not for that, but for keeping your data model consistent in a clean, understandable fashion.