Downcasting - Is it that bad?

[awesomeprograms]

I've been working on a 2D risk clone recently, and after a friend of mine reviewed my code he pointed out that downcasting should be avoided at all costs. My situation is as follows: I have a class called Entity that is the base for all graphical elements in the game. I have a class called Country which contains an enum that stores which player owns the country. This member, along with its get function, is not implemented in Entity, and it would not make sense to make an interface for this function, as Countries will be the only things that can be controlled by players. I also have a manager class to manage all of the entities in my game. The manager has a std::map which stores the entities. Since the manager holds objects of type Entity*, its getEntity() function will not give the returned object access to the getController() function. Because of this, I use a static_cast to downcast the returned Entity* into a Country*. Here's some code to represent what I'm doing:

enum Controller { Player1, Player2, Player3, Player4, Unclaimed };

class Entity {

public:

...

private:

...

};

class Country : public Entity {

public:

Controller getOwner();

...

private:

Controller owner;

...

};

class EntityManager {

public:

Entity* getEntity(std::string id);
void addEntity(std::string id, Entity* entity);

...

private:

std::map<std::string, Entity*> entityList;

};

int main() {

Country* country = new Country();

EntityManager manager;

manager.addEntity("country", country);

Controller owner = static_cast<Country*>(manager.getEntity("country"))->getOwner();

}

Is there a flaw in my design? What do you guys think? Thanks.

[dhayden]

EntityManager exists to handle graphical aspects of the game. That's fine but it doesn't mean that it is the sole gateway to your data. Create a Board or Game class that has the data you need. That class will contain the country and the EntityManager. The constructor adds the country to the EntityManager.

class Game {
public:
    EntityManager manager;
    Country country;
    Game() {
        manager.addEntity("country", &country);
    }
};

int main() {
    Game game;
    Controller owner = game.country.getOwner();
}

[awesomeprograms]

That's actually precisely what I do; I have a Map class that contains an EntityManager that holds all of the countries. I know that i can turn EntityManager into a template class to make what I'm doing easier, but downcasting is more straightforward in my eyes. Also, i use pointers to the objects, that way everything resides in the Entity Manager. Instances of these pointers are created in the constructor, then go out of scope, leaving handling of all Entity derivatives to the manager class. Because of this, i can't do map.country.*, since the pointer only exists in the manager class.

[awesomeprograms]

Does anybody have any input?

[Zarman]

Down casting puts you at risk of data loss
If you downcast from a double to an int, and your compiler reads doubles as 64 bits and ints as 16 bits, your compiler may error, or just try to interpret the first 16 bits of the double as the int type.

[dhayden]

That's actually precisely what I do

I don't think so because

Instances of these pointers are created in the constructor, then go out of scope, leaving handling of all Entity derivatives to the manager class

In my example, you can get to the country in two different ways: through the country member or through manager.getEntity("country");

I think the problem you're having is that you want to have one and only one way to get to the data. Sometimes it's handy to have multiple ways to get to it and this seems to be one of them.

Note that if you go with my method then you may have to deal with memory management if you find that the manager contains pointers to data that must be destroyed and data that must not.