“Correct” way to use inheritance

Question

Within a small project, a class Storage is meant to store any type of Item. Now, an Item has a String name, and an interaction. For example, a clock item might implement the interaction increment(int num), whereas a seat might implement sit(bool true).

Should I be simply writing classes for Item, Clock and Seat and letting Seat and Clock inherit from Item? Obviously the interaction they use varies, so short of writing every possible Item's interaction as a virtual method in Item I'm not sure how to deal with all the specific interaction types.

At the moment, I'm finding the Item and then using using conditionals to use the correct behavior on interaction, but that seems sort of roundabout and not so great.

Is there a better way to do this?

Answer 1

I think you are suffering from YAGNI in your design. Think about it like this - when would you ever interact with a clock and a seat in the same way? Answer: you wouldn't! You might name a clock, and name a seat, but you wouldn't sit on a clock, nor would you increment a seat. It doesn't make any sense.

Interfaces are for common behavior. For example, you might have an interface Named, which you would give to all implementations that have a Name. You might have an interface Timed, which might be given to all Events that occur at a specific time. Or you might have an interface Sittable, which would be implemented by Chair and Sofa.

If you really want to have an interface Interactable, you should either pass it something where the object itself can figure out what is doing the interaction, like Object[], or the object of what's being interacted with, such as Player.

Don't think about this sort of design in the beginning. Figure out what sort of interactions you are actually going to do, and figure out what the common behavior is going to be, and then design your interfaces at that level.

---

Response to comment:

when one interacts with a room, it uses that object (e.g. a Sofa or a Clock)'s interaction, how can I tell which it is with no prior knowledge?

This is what I was saying in the last paragraph. You put the behavior of the room in the code for the object itself, e.g. (Java syntax, but C# is very similar)

public class Bed implements Interactable {
  @Override
  void interact(World world, Player player) {
    player.addRest(10);
    world.advanceTime(8, TimeUnit.HOUR);
  }
}

Answer 2

If I understand you correctly, it sounds like you're working with something like this pseudocode:

void foo(Item i) {
   if(i instanceof Clock) {
       (i as Clock).increment();
   } else if(i instanceof Chair) {
       (i as Chair).sit();
   }
}

As you suspect, this is a code smell. A pretty strong one, actually.

The immediate problem is that, because this function has to know the concrete type of i, there is absolutely no benefit to using the Item base class. In fact, this function is claiming that it can take any arbitrary Item object, when it can't, so it's lying to everyone that tries to use it. If you're very lucky, the solution is as simple as splitting it into void foo(Clock c) and void foo(Chair c), but it may run deeper than that.

Normally, this happens when you design your object hierarchy around real-world things you want to "exist" in your program, rather than around the actions you actually want your program to perform. The only simple solution is to redesign the relevant part of your program. Unfortunately, without knowing anything about the rest of your program, we can only guess what the optimal change would be. On the one hand, maybe you can come up with a single Interactable interface that serves the needs of all of your objects and their callers. On the other hand, maybe it doesn't make any sense to try unifying all these disparate types and you should move the Chair code far away from the Clock code. And there are many additional options in between. Either way, it's going to be hard working out what the correct design is, but solving that problem now will save you a lot of pain down the road (unless of course this project stays very small and simple).

---

In the hopes of being slightly more useful than "this is a hard problem, but good luck", I'll try to sketch a simple example solution. Your example reminds me of video games that have an "interact" button, so let's say I'm designing the item interaction code for such a game. I could start by asking myself what keys, ammunition, first-aid kits, talismans and plastic bags have in common, but then I end up in the situation you described.

Instead I would ask what happens when I interact with each of these items. What I find is that, at least for the specific game I'm playing today, some of the possible answers include:

1. I want to remove the item from the screen, add it to the inventory, show a 3d model of it along with a "I got X" message, then wait for a button press.
2. I want to pause the game, show some text at the bottom of the screen, wait for the user to scroll through it by pressing buttons, then resume.
3. Like #2, except the last piece of text includes a choice, and depending on the user's selection, we may show an animation, change the state of some other objects in the room, and spawn a few monsters before resuming.

It would make lots of sense to write a standard interface for each of these item types, since all items of a given type do essentially the same things; you can write a function that takes a ShowTextInteractable which doesn't care whether it's a Book or a CreepyDoll or a Flowerpot (and it may not even use subclassing, perhaps new ShowTextInteractable("It's a flowerpot.") is good enough). Also notice that I haven't even touched on using items from the inventory; those should be completely different types from items "in the world" because they do completely different things. But hopefully that gives you some idea what I'm talking about.

Answer 3

Another approach would be to use composition instead of inheritance. We can look to the command pattern for inspiration. We would like to have a common interface while allowing variable actions. Rather than write the behavior into the Item itself (as durron597 does), we could instead create an intermediary object to hold the interactive behavior we want. Let's start with the interface:

interface Interaction {
    void interact(/** things to pass in **/);
}

Now let's create an implementation:

class ClockInteraction implements Interaction {
    private Clock clock;

    public ClockInteraction(Clock clock) {
        this.clock = clock;
    }

    public void interact() {
        clock.increment(1);
    }
}

And another:

class SeatInteraction implements Interaction {
    private Seat seat;
    private boolean shouldSit;

    public SeatInteraction(Seat seat, boolean shouldSit) {
        this.seat = seat;
        this.shouldSit = shouldSit;
    }

    public void interact() {
        seat.sit(shouldSit);
    }
}

The behavior of the Interaction is now separate from the behavior of the Item. Does this provide us with any benefits beyond implementing the behavior directly into the Items? First, we can create multiple behaviors for the same object type:

class ClockReverseInteraction implements Interaction {
    private Clock clock;

    public ClockReverseInteraction(Clock clock) {
        this.clock = clock;
    }

    public void interact() {
        clock.increment(-1);
    }
}

Now I have two different behaviors that can be used on the same object, and I can choose between them at runtime.

We can also compose together multiple Items into a single behavior:

class SitWhenInteraction implements Interaction {
    private Clock clock;
    private Seat seat;

    public SitWhenInteraction(Clock clock, Seat seat) {
        this.clock = clock;
        this.seat = seat;
    }

    public void interact() {
        seat.sit(clock.value() < 10);
    }
}

Now, rather than Storage hold Items, it should hold Interactions instead:

foreach(Interaction interaction : storage) {
    interaction.interact();
}