3

Preamble: I am making a simple board game in C++, in which AI characters move around squares on the board. There are different types of squares, each inherited from an abstract class, each with different effects on the AI character. For example, some are shops where the character can buy stuff, others are workplaces where characters get money, etc.

Likewise, there are different AI characters (also inherited from an abstract class), which make different kind of decisions. Some work more than others, some eat more than others, etc.

The problem is that the AI character needs to know the square type in order to make a decision about what to do. At the moment checking the type using a type-name string as a member variable is the easiest solution, but it feels horribly inelegant and not very helpful when it comes to adding new types of squares and AI characters.

Question: Is it bad practice to check the type of an object by asking it for an identifier which is placed into its class for this purpose? Is it bad practice to use other methods of run time type identification such as dynamic_cast and typeinfo for purposes of controlling the flow of execution?

Example: Below the Square class (accessed through currentSquare() has a member function getType() which returns a string with the name of the type:

//Normal AI character
void decide() {
    if(currentSquare()->getType()=="HOME") {
        watchTV();
        sleep();
    } 
    if(currentSquare()->getType()=="WORKPLACE") {
        work();
    } 
}

//Party animal AI
void decide() {
    if(currentSquare()->getType()=="HOME") {
        moveTo(getGameBoard()->closestBar());
    } 
    if(currentSquare()->getType()=="WORKPLACE") {
        sleep();
    } 
}
  • Why not explicitly use the Visitor pattern? – amon Oct 14 '13 at 20:57
  • @amon personally, every time I try to use the visitor pattern the code becomes harder to maintain. I'm a java programmer though, maybe that makes a difference. – Daniel Kaplan Oct 14 '13 at 21:37
  • @tieTYT I'm not that fond of visitors myself, but they do provide more type safety than an if/else, and deal with inheritance better than a manual dispatch table (of which OP's code is a flattening). True multidispatch would be a neat thing to have. – amon Oct 14 '13 at 21:46
13

As Amon pointed out, this is a good application for the visitor pattern. Using it, your AI classes will end up looking something like this:

void decide(HomeSquare square);
void decide(WorkSquare square);
void decide(ShopSquare square);

And your squares have an accept function that looks like:

void accept(AI ai)
{
    ai.decide(this);
}

That lets you use inheritance to create appropriate defaults, or force every AI to implement certain decisions. You can create an AI that behaves exactly like another one except at home, for example, without copying a bunch of code. The compiler will point out where you forget something, and you don't have to cast all over the place.

3

I would like to point out the fragility of using strings for identifying classes of objects. You are somewhat working around type safety and if something goes wrong you may receive weird errors at run time. If you have quite some classes, you want to check, it can become a pain to update all parts of the code if you introduce a new class or delete another. Moreover it complicates or even prevents static analysis thus reducing the likelihood of automatically finding bugs (like dead code).

Compare this to using the visitor pattern that others already pointed out. You will get type safety and (if something goes wrong) compile time errors. On top of this the visitor pattern gives you more flexibility and you won't get long cascaded if-else blocks or switch blocks, thus improving readability and maintainability. Static analysis tools will work as intended.

So I would argue that using member variables for identification is at the very best suboptimal.


Full disclosure: I'm mainly a java developer but I think in this case the differences between java and c++ are not a concern. If I am wrong someone please correct me. ;)

2

I prefer to use enums rather than strings for situations like this, and the "check type" code should be written to exclude the unspecified cases.

switch (currentSquare()->getType())
{ default: BUG("case not expected");
  case workplace: ...
    break;
  case home: ..
    break;
}
  • 1
    I will implement your suggestion, but it doesn't really answer my question directly. Should I infer that you see it as OK design to check an object's type through a member variable? – Atuos Oct 14 '13 at 20:05
  • I had no idea default could be placed at the head and work, that's interesting. – Thomas Eding Oct 14 '13 at 21:26
  • sometimes it's the best choice, especially if the behavior is not strictly linked to the class. There's definitely tension between defining behavior by class inheritance verses other means. – ddyer Oct 14 '13 at 21:28

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.