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:
- 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.
- 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.
- 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.
object
class, then there is no need to invent an ownItem
class just for filling a "mixed type" container with different objects. – Doc Brown Jul 27 '15 at 8:00