Object-oriented design/programming questions
Jeremy Murphy
Hi everyone,
I've just been perusing the articles and am so happy to discover that there
is a friendly, active community for this niche genre! I may not be active
for long since I'm a bit of a wanderer, but let's see.
After teaching myself C++ recently, I thought writing a roguelike would be a
great exercise to test my new knowledge. So far, I have written simple
dungeon and creature objects such that I can move the player and monsters
wander randomly. The problem I am facing is with managing the complexity of
the Creature class and its descendants.
I would like to have an abstract Creature class that is inherited by a
Simple creature class, which is then in turn inherited by a Complex creature
class. Simple creatures can move, attack and not much else. Complex
creatures can use items, spells, etc. Has this paradigm of creature
inheritance proven useful (or painful) before?
Anyway, I will assume that it is feasible and persist. Since a Complex
creature can get() an item, etc but a Simple creature cannot, I assume that
I have to declare all the Complex behaviour functions as virtual in the base
Creature class. Or do I? Can I incrementally add to my creature subclasses
instead? Since a Simple creature cannot get() and thus would not try to,
why does it need an empty virtual function? The difficulty I have
encountered in pursuing this design is in how to get actions from a
creature:
switch (creature->action())
{
case MOVE: ...
case ATTACK: ...
case GET: creature->get(); // ERROR for Simple creatures
}
So, is there an elegant solution to fix my calling routine that allows for
incremental change to Creature subclasses, or do I need to bite the bullet
and just declare all behaviour as virtual in the base class so that I can go
ahead and treat all Creature pointers the same way?
Thanks in advance! Cheers.
Jeremy
simenendsjo
When starting OOP, it's easy to try to model everything through
inheritance. This leads to a maintenance nightmare as everything has a
very strong relation on the hierarchy. Reading your post also makes me
believe you have almost an god class there.
You should try to model the classes so they only have _one_ distinct
function (search for "single responsibility principle"), and these
classes makes up the larger ones by composition (search for "favor
composition over inheritance").
A simple example:
interface ICreatureBehavior
{
HandleAction(Action);
}
class ComplexCreatureBehavior : ICreatureBehavior
{
HandleAction(Action action) {
// handles everything
}
}
class SimpleCreatureBehavior : ICreatureBehavior
{
HandleAction(Action action) {
// get doesn't work
}
}
class Creature
{
// Notice we give the behavior here. Then we can change behavior at
runtime if for instance it get's zapped with a "wand of can't pickup"
Creature(ICreatureBehavior behavior) ...
}
Paul Donnelly
> I would like to have an abstract Creature class that is inherited by a
> Simple creature class, which is then in turn inherited by a Complex creature
> class. Simple creatures can move, attack and not much else. Complex
> creatures can use items, spells, etc. Has this paradigm of creature
> inheritance proven useful (or painful) before?
I think most people here will say that a simpler hierarchy works out
better.
> Anyway, I will assume that it is feasible and persist. Since a
> Complex creature can get() an item, etc but a Simple creature cannot,
> I assume that I have to declare all the Complex behaviour functions as
> virtual in the base Creature class. Or do I? Can I incrementally add
> to my creature subclasses instead? Since a Simple creature cannot
> get() and thus would not try to, why does it need an empty virtual
> function? The difficulty I have encountered in pursuing this design
> is in how to get actions from a creature:
>
> switch (creature->action())
> {
> case MOVE: ...
> case ATTACK: ...
> case GET: creature->get(); // ERROR for Simple creatures
> }
>
> So, is there an elegant solution to fix my calling routine...
Getting rid of Simple creatures. ;) Look at the problem this way: a
class should be designed to express its role in the program, not in the
real world or in the world modeled. Simple creatures express the
latter. It's true (but irrelevant) that there are creatures which aren't
supposed to be able to pick up items, but a creature that isn't supposed
to won't try, will it? There's no reason to complicate the class
hierarchy for this.
Since you're trying to treat all Creatures the same, it's a design error
to make them behave differently in situations where they should be
interchangeable. Calling get() on a creature that won't get shouldn't be
an error that disrupts normal program flow. A get() method that fails
cleanly for creatures that can't get, by getting no item and logging the
attempt (if that's considered an AI bug), will probably be easier to
work with. You could technically implement that get() method as a
virtual function, with different implementations for Simple and Complex
creatures, but it will probably be much less work for you in the long
run to differentiate creatures only through parameters that can be
loaded from a data file, and not with classes at all.
If you really need to indicate whether a Creature is allowed to get(),
you could add a "can get" flag to the class and check it in a
non-virtual get() method. You may find it useful to subclass your AI to
control which creatures will do what; since you will probably have far
fewer types of AIs than creatures, it won't be so unmanageable. AI
classes will provide some of the behavioral guarantees you may be
looking for.
Kenneth 'Bessarion' Boyd
> Hi everyone,
>
> I've just been perusing the articles and am so happy to discover that there
> is a friendly, active community for this niche genre! I may not be active
> for long since I'm a bit of a wanderer, but let's see.
>
> After teaching myself C++ recently, I thought writing a roguelike would be a
> great exercise to test my new knowledge. So far, I have written simple
> dungeon and creature objects such that I can move the player and monsters
> wander randomly. The problem I am facing is with managing the complexity of
> the Creature class and its descendants.
And that's before you try to write your save/load implementation. (I
missed the ARRP by virtue of a work schedule flare, but basically
regardless of one's hierarchy it is obnoxiously difficult to make
anything other than non-pointer Plain Old Data work in savefiles.)
> I would like to have an abstract Creature class that is inherited by a
> Simple creature class, which is then in turn inherited by a Complex creature
> class. Simple creatures can move, attack and not much else. Complex
> creatures can use items, spells, etc. Has this paradigm of creature
> inheritance proven useful (or painful) before?
The following inheritance diagram looks more reasonable to me:
Creature --> SimpleCreature
|
v
ComplexCreature
Handle the risk of code/data member duplication by moving duplicate
code, data members, etc. to the abstract class Creature.
Please don't be blinded by the few actions the two have in common.
You're going to have to write two different AIs anyway (at least).
> Anyway, I will assume that it is feasible and persist. Since a Complex
> creature can get() an item, etc but a Simple creature cannot, I assume that
> I have to declare all the Complex behaviour functions as virtual in the base
> Creature class.
Only if you refuse to properly fork your functions. It's tempting.
Functions that make sense only for ComplexCreature, should be defined
only for ComplexCreature.
> ... The difficulty I have
> encountered in pursuing this design is in how to get actions from a
> creature:
>
> switch (creature->action())
> {
> case MOVE: ...
> case ATTACK: ...
> case GET: creature->get(); // ERROR for Simple creatures
>
> }
Well, yes if you use the wrong level of abstraction you get locked
into this. This switch statement should be within both SimpleCreature
and ComplexCreature's implementation of a common virtual function
(which may well be a candidate for pure virtual). [FATAL is presumed
to emit a text message, then exit with status 1; it's a no-return
function.]
The above switch should simply be
creature->act()
with example implementations:
SimpleCreature::act()
{
switch(action())
{
default: FATAL("invalid action");
case MOVE: ...
case ATTACK: ...
}
}
ComplexCreature::act()
{
switch(action())
{
default: FATAL("invalid action");
Krice
> The problem I am facing is with managing the complexity of
> the Creature class and its descendants.
Good rule is to avoid inheritance unless it's making things
easier. You don't have to inherit to create "sensible"
looking class hierarchy. For game objects I think
base class+one derived for each object type (item, monster,
door, etc.) is all you need.
J. Prevost
Another way to think about "making things easier" is to start building
individual sorts of things and then put them together in an
inheritance hierarchy only when you see a good place where you can
share code.
Consider, in the specific case of "creatures", that most parts of your
program that work with creatures don't want to be specialized to a
specific kind. You're not going to have functions or lists that can
*only* hold four-legged creatures, for example. That means that
you're not going to be using the subtyping part of subclassing very
much, which pretty much means that you're free to subclass only when
it makes sense to share behavior.
For example, if you have a few different creatures that exhibit a
common behavior (calling for help, say) then you might notice that
when you go to make the second kind, and make a new superclass to
share the "call for help" code. That way if you notice a bug in the
logic for that feature, you only have to fix it in one place, instead
of having to hunt down every place you've copied and pasted the "call
for help" code.
But most importantly: keep things easy and do what seems reasonable.
Don't make things elaborate without a reason for it. There'll be
plenty of time to do things in better or more advanced ways when
you're more comfortable with the basics.
Nik Coughlin
> Hi everyone,
>
> I've just been perusing the articles and am so happy to discover that there
> is a friendly, active community for this niche genre! I may not be active
> for long since I'm a bit of a wanderer, but let's see.
>
> After teaching myself C++ recently, I thought writing a roguelike would be a
> great exercise to test my new knowledge. So far, I have written simple
> dungeon and creature objects such that I can move the player and monsters
> wander randomly. The problem I am facing is with managing the complexity of
> the Creature class and its descendants.
>
> I would like to have an abstract Creature class that is inherited by a
> Simple creature class, which is then in turn inherited by a Complex creature
> class. Simple creatures can move, attack and not much else. Complex
> creatures can use items, spells, etc. Has this paradigm of creature
> inheritance proven useful (or painful) before?
http://stackoverflow.com/questions/49002/prefer-composition-over-inheritance
Konstantin Stupnik
> inheritance. This leads to a maintenance nightmare as everything has a
> very strong relation on the hierarchy. Reading your post also makes me
> believe you have almost an god class there.
>
> You should try to model the classes so they only have _one_ distinct
> function (search for "single responsibility principle"), and these
> classes makes up the larger ones by composition (search for "favor
> composition over inheritance").
I'll add that by doing so you can easily do things like:
Stupefy monster by some effect simply by temporary replacing it's AI.
Make some kind of 'Swarm' AI, where group of monster are sharing
the same instance of an AI object, which makes their behavior
more synchronized.
Gerry Quinn
says...
> Anyway, I will assume that it is feasible and persist. Since a Complex
> creature can get() an item, etc but a Simple creature cannot, I assume that
> I have to declare all the Complex behaviour functions as virtual in the base
> Creature class. Or do I? Can I incrementally add to my creature subclasses
> instead? Since a Simple creature cannot get() and thus would not try to,
> why does it need an empty virtual function? The difficulty I have
> encountered in pursuing this design is in how to get actions from a
> creature:
>
> switch (creature->action())
> {
> case MOVE: ...
> case ATTACK: ...
> case GET: creature->get(); // ERROR for Simple creatures
> }
As others have pointed out, this is probably a bad idea.
The action above is probably coming from Creature::GetAction() anyway,
so simple creatures can just be creatures with an AI that never chooses
anything other than MOVE or ATTACK.
Creatures don't have to use all their capability.
- Gerry Quinn
simenendsjo
Exactly. Single Responsibility and Dependency Injection makes flexible
systems that are easy to test, understand, change and extend.
It's nearly a silver bullet if done right.
Jeremy W. Murphy
Thanks everyone for your replies. They confirmed that I have fallen prey to
the temptation of hitting everything with the inheritance stick, which I had
already read about in a few places.
Let's put the Creature somewhat to the side for a moment. The way I've
designed things is that a Level has a grid of Tiles, and each Tile can have
a stack of items and a Creature. This design got awkward rather quickly,
because Creatures don't know about Tiles, so any time a Creature wants to
move or get an item, or do anything non-trivial, it has to pass a message
back 'up' to the Tile that then facilitates the action. Is this typical?
It feels like the code for the Tile and Creature are going to be very
tightly linked, giving rise to the kind of issue that I raised in my
previous email: the Tile having to know about different types of Creatures
and their actions. Cheers.
Jeremy
Gerry Quinn
jeremy.wil...@gmail.com says...
>
> Thanks everyone for your replies. They confirmed that I have fallen prey to
> the temptation of hitting everything with the inheritance stick, which I had
> already read about in a few places.
>
> Let's put the Creature somewhat to the side for a moment. The way I've
> designed things is that a Level has a grid of Tiles, and each Tile can have
> a stack of items and a Creature.
Pretty standard.
> This design got awkward rather quickly,
> because Creatures don't know about Tiles, so any time a Creature wants to
> move or get an item, or do anything non-trivial, it has to pass a message
> back 'up' to the Tile that then facilitates the action. Is this typical?
Not really. Unless you mean something like calling Tile::GetItem() or
whatever. Creatures deciding what to do will normally have access to a
map of the area around them, which will typically be an array of tiles.
Probably there's no reason why there should not be a static pointer to
this map - after all there is usually only one level at a time.
Creatures shoulds not be owned by tiles: they should be owned by the
Map at minimum, or the Level (which owns the Map).
So...
void CPoint Creature::FindTarget()
{
for ( int x = m_x - m_Eyesight; x <= m_x + m_Eyesight; x++ )
{
for ( int y = m_y - m_Eyesight; y <= m_y + m_Eyesight; y++ )
{
if ( m_pMap->GetTile().IsMonster() )
{
if ( m_pMap->GetTile().GetMonster().IsEnemy() )
{
return CPoint( x, y );
}
}
}
}
return CPoint( -1, -1 );
// or have function return a boolean, if you prefer
}
Is some information stored twice? Yes! Object orientation makes this
easy to organise safely by ensuring that all copies are updated at
once, perhaps in Map::MoveCreature(), which updates both the creature's
m_x and m_y coordinates, and the references to the creature on the map
tiles it is leaving and entering.
[Actually Creature will store a CPoint instead of m_x and m_y, I just
wrote it as above for ease of understanding.)
> It feels like the code for the Tile and Creature are going to be very
> tightly linked, giving rise to the kind of issue that I raised in my
> previous email: the Tile having to know about different types of Creatures
> and their actions. Cheers.
Tiles don't nreed to know anything about creatures, except that they
probably have a Creature pointer or reference or ID.
- Gerry Quinn
Kenneth 'Bessarion' Boyd
<jeremy.william.mur...@gmail.com> wrote:
> .... The way I've
> designed things is that a Level has a grid of Tiles, and each Tile can have
> a stack of items and a Creature.
Ok.
> This design got awkward rather quickly,
> because Creatures don't know about Tiles, so any time a Creature wants to
> move or get an item, or do anything non-trivial, it has to pass a message
> back 'up' to the Tile that then facilitates the action. Is this typical?
Yes. (I would isolate the Tile collection inside a Map.) The way my
vaporware works around this, is....
/*
* ownership issues
* Agent -> View [1:1]
* View -> Map [1:1]
* Map -> View [many:1]
*
* when agent loses view (agent death/scenario shutdown)
* * View is double-linked at start
* * View tells Map to deregister it (goes to single owner Agent)
* * Agent deletes View
* when agent gains view
* * Map creates View for Agent
* Map shutdown: delegate to agents
*
* Thus:
* * delegate the deregister request to View's destructor
* * view constructor requires all of: map,agent id,default origin
*/
[I got stuck with agent id because nothing less would be stable intra-
game; this does mean I'm stuck with binary search rather than array
dereference in a number of places.]
Convoluted. While the rendering engine does have to know the current
viewpoint Agent, the rendering engine and the View class are the only
things that have to be substituted at link-time to convert between
console and graphical interfaces. (In particular, the map is not
directly exposed to either the Agent, or the rendering engine. The
heavy lifting is in View.)
It's View's responsibility to translate player/AI commands into actual
Map state changes. For movement, this is updating the Agent-to-
position function that Map maintains.