Removing a base class from its according collection OOP

Question

Lets say i have a zoo class, with a bunch of collections. The collections are giraffes, elephants, tigers etc. All animal types derive from the same interface or base class, lets call it IAnimal.

So in my code i'm running in the same problem in 2 places. I end up in a method, that gets an IAnimal that its supposed to delete, but i dont know which exact type it is. So i basically end up with methods that look like this

if(animal is Tiger tiger)
   Zoo.Tigers.Remove(tiger);
   TigerRepository.Delete(tiger);
else if(animal is Giraffe giraffe)
   Zoo.Giraffes.Remove(giraffe);
   GiraffeRepository.Delete(giraffe);
....

This code moves around, sometimes it lands in the zoo class, sometimes in a DBService class, sometimes in a viewmodel. Sometimes its a dictionary, but lets be honest its the same thing.

I know i can probably brute force a solution that i never need to touch again with some smart reflection somehow, but that feels like its not really OOP.

I can split up the method it ends up in from a single method that gets an IAnimal into several method that get a Tiger, Giraffe etc, but that feels the same as a big factory method. It feels like i should be able to do something with generics, but it just doesn't work since i don't know the type at compile time.

The best i can do is a bunch of DeleteTiger(Tiger tiger) methods that internally call a DeleteAnimal<Tiger> method that saves a single line of code in each of the delete methods.

This isn't just a theoretical best practice thing, i keep having to go back and back to that method every time i add a new subentity to my application.

What can i do to stop having to do this? Can i even do anything?

Edit: I don't like the just make an animal collection approach, but it would definitely work with in memory objects. I would never get the idea to model a zoo as "300 animals", its 20 monkeys, 5 tigers... etc.

However how would this approach work with the repositories or entity frameworks dbsets WITHOUT having a method that just switches on the type again takes that repository / dbset and calls the Delete method?

I just saw that EF Cores DBContext actually has just simple Remove(object entity) method, but internally it probably just type checks too. I feel like i cant get around it with a repository pattern approach.

Answer 1

The comments by Rik D and amon already point out two possible solutions that come to mind:

• If all items are instances of subclasses with a common superclass or interface, why keep them in separate collections? You might put them into just one and use predicates such as isTiger() to filter when you want to handle the tigers only.
• If these are fundamentally different items, you shouldn't normally mix them in a function signature. Instead of remove(x) where x may be an animal or a building, implement removeAnimal() and removeBuilding(). Implementing the visitor pattern on top of that isn't really hard: have each item class implement a removeFrom(zoo) method calling the item-specific remove...() method of the zoo, and call that from the generic remove() method.

Which variant better fits most likely depends on the actual case. Both are proper OOP.

Answer 2

The trouble is your animals are modeled with polymorphism but your stores are not.

If an animal would get a reference to its store at construction time, the animal could have a Delete method that calls this.Store.Delete(this.id);.

If you do not want animals to delete themselves or have that reference, you can have your animal manager call Delete on the polymorphic store: store.Delete(animal.Id);

Only the store instance would know the type of animal at delete time, no one else needs or wants to know. There will be no type checking in your different polymorphic stores, each will deal with a specific known type.

Answer 3

My approach here would be to maintain a single collection of IAnimal and a single Zoo.Remove() method. Then when you need just the elephants, have Zoo.getElephants().

Answer 4

Lets say i have a zoo class, with a bunch of collections.

That's it, you're already on for a wild ride if you go that route. Do you have any good reason to model your repository (or the abstraction thereof, at least) as an in-memory abstraction? Have you considered Query Objects (see also here)?

Edit: I don't like the just make an animal collection approach, but it would definitely work with in memory objects. I would never get the idea to model a zoo as "300 animals", its 20 monkeys, 5 tigers... etc.

Now there's the heart of your problem. See, when you designate an IAnimal interface, as you have done, it expresses your intent (to me, who's reading your code, at least) as "I want to treat all animals the same way, and leave implementation details aside". Why do you have animals implement an IAnimal interface, but then would never get the idea to model a zoo as "300 animals"?

However how would this approach work with the repositories or entity frameworks dbsets WITHOUT having a method that just switches on the type again takes that repository / dbset and calls the Delete method?

Martin Maat has a marvelous solution for you: Generalize based on Identifiers rather than Types. To your stores, everything looks the same way, it's fields of tables. Make sure that your IAnimals (you do want to treat all animals the same, don't you?) are all unique in your stores (no overlapping IDs, that is), and then make an abstraction akin to Delete(int) and you should be good to go.

By using a typed repository like Zoo, you are effectively trying to pretend you have polymorphic stores, when you're not, as Martin Maat aptly puts it!