C++ Design – Is it Bad Practice to Check Object Types with an Identifying Member Variable

cdata typesdesignobject-orientedpolymorphism

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();
    } 
}

Best Answer

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.

Related Solutions

Encapsulation – How Encapsulation Enhances Safety

How will encapsulation help when making changes in code and from its rippling effects. For a data member, if I change its type from int to float, (even if I am exposing this using property) I will need to change variable type where I am using already using this code.

The benefit of encapsulation is that it lets you change the internal implementation without breaking client code. It doesn't protect you if you decide that you need to change the interface to your code, but that's a different matter.

Example: Say you have a value representing the price per unit of some commodity. The price is expressed in cents, and because you don't deal in fractional cents you decided to make the property an integer (I'll use C here because I'm not very familiar with C#):

int _price

int pricePerUnit(void) {
    return _price;
}

int priceForUnits(int units) {
    return units * _price;
}

That all works out fine until one day when somebody notices that your firm is losing a lot of money due to rounding errors. Many of the commodities that you track are bought and sold in lots of many thousands of units, so you need to start tracking the price to an accuracy of at least 0.001 cent. Because you were smart enough to encapsulate the price instead of letting clients access it directly, you can make that change pretty quickly:

double _dprice

int pricePerUnit(void) {
    return (int)_dprice;
}

int priceForUnits(int units) {
    return (int)(units * _dprice);
}

The interface that clients use to obtain prices stays the same, but the data they get back is now more accurate. If the price per unit is $1.001, priceForUnits(1000000) will now return a price that's $1000 greater than before. That happens even though you haven't changed the interface to your system at all, and you therefore haven't broken any client code.

Now, that may not always be all that you need to do. Sometimes you'll need to change or augment your interface so that you can report the price more accurately to clients, too:

double pricePerUnit() {
    return _dprice;
}

A change like that will break client code, so you might instead keep the old interface and provide a newer, better routine:

int pricePerUnit() {
    return (int)_dprice;
}

double accuratePricePerUnit() {
    return _dprice;
}

You and the rest of your team can then embark on the process of converting all the clients of your system to use the newer, better accuratePricePerUnit(). The client code will get more accurate as you make progress on that task, but even the old stuff should continue to work as well as it did in the past.

Anyway, the point is that encapsulation lets you change the way the internals work while presenting a consistent interface, and that helps you make useful changes without breaking other code. It doesn't always protect you from having to update other code, but it can at least help you do that in a controlled manner.

C++ Class Design – Data Classes: Getters and Setters or Different Method Design?

Part 1

This is a good design question. You are correct in detecting code smell concerning getters and setters. They generally indicate a design problem exposing the implementation details of your object.

Try to think in terms of what your objects should do - Tell, Don't Ask:

Your first problem may be that you are trying to design "data classes". Rather than worry about the data (implementation details), think about the functionality. Again, what should your objects do? In your case, what do you want to do with character styles? Who (software-wise) cares about character styles? What do they need to do?

Hopefully that gets you started. Test driven development helps with these kinds of design problems. It forces you to think in terms of function not data.

On the contrary, if all you need is a data container, write a C-style struct class and go to town. I wouldn't recommend that, maintaining it will be a $@#%#.

Good luck!

Part 2

What you want is either a simple data class - skip the getters and setters completely, or you want to create a platform independent abstraction that places a facade (Facade Pattern) in front of the rendering and style setting. It simply provides an interface for setting styles and rendering. Your platform specific implementation does the dirty work (using NSTextView in your example).

The benefit of the simple data class is that it is initially simple to write. Its drawback is that you will have a hard time avoiding a giant tangle of if-else statements. You will also be lacking a clear place to make you platform specific rendering calls that use the styles. As the system's complexity grows, you may find it more diffcult to decide where implementation details go.

The facade is a more abstracted approach. The benefits are that it is more flexible and can be reused if you decide to port to another platform. Its drawbacks are more upfront development time.

The facade's public interface will provide what you need to set and remove styles as well as initiate rendering when the time comes.

The details of how you want to set styles are up to you. Use what ever system feels best. Simple getters and setters or a generic set and get that uses a dictionary internally works too (see boosts ptree if you are using C++). You could even take all (or default) styles at construction time. You could decide to not even expose mutators at that point. Your call. Perhaps you decide it is important to data drive the styles you support and use a configuration plus factory system (We can add more detail later if that is important to you). In fact different implementatios of the facade could provide different ways of approaching the problem. You could prototype a few and choose what works best.

The platform specific implementation of you facade abstraction will use the platform specific rendering system (NSTextView in your case) and the styles you have set to make the appropriate calls to the system. Simply inject the platform specific classes at construction (Dependency Injection), implement your render() method and you should be good to go.

Part 3

If your system design allows, you could take all styles for a particular element at construction time. This could allow you to avoid getters and setters completely if you chose to make your element immutable. Your would then have a simple, clean and possibly immutable abstraction in front of your character styles system. Immutable state generally leads to fewer bugs but does require you to operate under the premise that you cannot change things willy-nilly.

Taking this a step further, a configuration file might define the different style setups you have. Again this would require prior knowledge of what styles you are setting (similar to constructing with the styles above). Give the type of style you are seeking, say "heading", you might fetch the configuration for headings which specifies a larger, bold font.

These are just some ideas off the top of my head. Without further requirements gathering and use cases it will be tough to get more specific.

Hope that helps. Good luck!