Is my class structure good enough?

Question

So I wanted to try out this challenge on reddit which is mostly about how you structure your data the best you can. I decided to challenge my C++ skills. Here's how I planned this.

• First, there's the Game class. It deals with time and is the only class main has access to.
• A game has a Forest. For now, this class does not have a lot of things, only a size and a Factory. Will be put in better use when it will come to SDL-stuff I guess
• A Factory is the thing that deals with the Game Objects (a.k.a. Trees, Lumberjack and Bears). It has a vector of all GameObjects and a queue of Events which will be managed at the end of one month.
• A GameObject is an abstract class which can be updated and which can notify the Event Listener
• The EventListener is a class which handles all the Events of a simulation. It can recieve events from a Game Object and notify the Factory if needed, the latter will manage correctly the event.

So, the Tree, Lumberjack and Bear classes all inherits from GameObject. And Sapling and Elder Tree inherits from Tree.

Finally, an Event is defined by an event_type enumeration (LUMBERJACK_MAWED, SAPPLING_EVOLUTION, ...) and an event_protagonists union (a GameObject or a pair of GameObject (who killed who ?)).

---

I was quite happy at first with this because it seems quite logic and flexible. But I ended up questionning this structure. Here's why :

1. I dislike the fact that a GameObject need to know about the Factory. Indeed, when a Bear moves somewhere, it needs to know if there's a Lumberjack ! Or it is the Factory which handles places and objects. It would be great if a GameObject could only interact with the EventListener... or maybe it's not that much of a big deal.
2. Wouldn't it be better if I separate the Factory in three vectors ? One for each kind of GameObject. The idea would be to optimize research. If I'm looking do delete a dead lumberjack, I would only have to look in one shorter vector rather than a very long vector.
3. Another problem arises when I want to know if there is any particular object in a given case because I have to look for all the gameObjects and see if they are at the given case. I would tend to think that the other idea would be to use a matrix but then the issue would be that I would have empty cases (and therefore unused space).
4. I don't really know if Sapling and Elder Tree should inherit from Tree. Indeed, a Sapling is a Tree but what about its evolution ? Should I just delete the sapling and say to the factory to create a new Tree at the exact same place ? It doesn't seem natural to me to do so. How could I improve this ?
5. Is the design of an Event quite good ? I've never used unions before in C++ but I didn't have any other ideas about what to use.

---

Well, I hope I have been clear enough. Thank you for taking the time to help me !

Answer 1

1. If I understand your design, there is no need for the GameObject to know the Factory type. The only case where a GameObject would need to access a factory is when it needs to create another GameObject, which is common for objects that spawn other objects. However, since you are already using an event system, you can just define an event type such as "create object". This way, the GameObject type can spawn/create other objects by firing an event, without knowing about the existence of a Factory.

2. No, not a good idea to separate the GameObjects by "type", GameObject is supposed to be an abstraction, if you start typing it, you loose that abstraction and create coupling. Also, this would not be scalable if you where to introduce another specialized GameObject "type". It would require changing the factory to add another container for this new type. OOP-wise, it is best to keep the GameObject abstract.

3. You can optimize search and management of the GameObjects by using a more efficient data structure. A scene graph or a quadtree can reduce the number of queries when dealing with a lot of GameObjects spread into a given space. Also, for the specific case of removal, you can delegate that task to the GameObject itself. When the object is expired, make it fire an event: E.g.: new Event_RemoveObject(this) that passes itself as the event argument. When the event is handled, the event manager will know who to delete.

4. If you want to design for scalability, it is best to avoid inheritance. Taking the Tree case, other more specific kinds of trees don't need to be objects in their own right. You can use configuration parameters to define that. This is what is otherwise known as Component Based Design. Define the behavior of a GameObject from the set of its components. This makes for a very flexible and extensible approach.

5. Yes, you can use a C++ union to define a Variant type.

This is an example I have used before:

union Data {
    void *    asVoidPtr;
    bool      asBoolean;
    int       asInteger;
    float     asFloat;
    char *    asCString;
    Vector3 * asVector3;
    Vector4 * asVector4;
};
// Plus an enum flag that tell which field of the
// union is in use, e.g.: a "type tag"

It should be enough if the parameters of your events are only primitive types and raw pointers. If you need more complex parameters like objects and smart pointers, then your best bet is to define an abstract Event class and specialize it as needed.