C++ Code Organization / Refactoring

Started by
11 comments, last by signal_ 15 years, 3 months ago
I need some advice on how to organize my code. Specifically, I have a creature in my game; I am building this generic class so that I can learn how to do more specific things to add to my game world. I am using C++. I apologize in advance for the verbosity, but I wanted to explain my situation as thoroughly as possible so that I might obtain useful advice. Thanks for reading. So let's say I have a class (class.cpp):

#include "creature.h"

class Creature
{
     private:
          // private member variables, etc here.
          int xCo;  // creature's x-coordinate.
          int yCo;  // creature's y-coordinate.

     public:
          // member functions here.
          // member functions for AI.
}
I set this class up to be basic: it moves in the game world and its position is recorded using private member variables xCo and yCo. The creature is animated. It responds to collision detection. It has a wandering member function. It is updated and rendered; all the basic stuff. Then I added A* path finding. Path finding (for my code), so far only needs two sets of data: 1) where the creature currently is (xCo, yCo) and 2) where it is going. I created two more files: pather.h and pather.cpp. I then modified the creature class to:

#include "creature.h"
#include "pather.h"

class Creature
{
     private:
          // private member variables, etc here.
          int xCo;        // creature's x-coordinate.
          int yCo;        // creature's y-coordinate.
          Pather pather;  // create path finding interface.

     public:
          // member functions here.
          // member functions for AI.
}
Now the pather object, in my code, only needs access to the Creature class private member data int xCo and int yCo, so I just passed that when calling the pather functions. Now I am refactoring the AI portion of the Creature class code. I wish to create another class, called AI, and utilize it in a similar manner as the aforementioned Pather class. So I created AI.h and AI.cpp. The Creature class code can then be modified to:

#include "creature.h"
#include "pather.h"
#include "ai.h"

class Creature
{
     private:
          // private member variables, etc here.
          int xCo;        // creature's x-coordinate.
          int yCo;        // creature's y-coordinate.
          Pather pather;  // create path finding interface.
          AI ai;          // create artificial intelligence interface.

     public:
          // member functions here.
          // member functions for AI no longer are here.
          // these functions are in the AI class.
}
Here is my overall organization question: I am refactoring the already existent AI code from the Creature class. I put all of the member variables that only AI functions use as private member variables in the AI class. However, unlike the Pather class which only used the Creature class's private variables xCo and yCo, the AI class needs access to a considerably larger portion of the Creature class's private variables. What is the best way for the AI class to obtain access to these variables? I could use get() and set() member functions, friend class, or ???? What can I do here? What is the wise way in which to organize this code? Thanks in advance for any suggestions.
Advertisement
Quote:What is the best way for the AI class to obtain access to these variables?


The AI class shouldn't have access to the creature's variables. Instead, the creature should provide functions that the AI class would call, and then the function would determine whether or not the creature can do whatever the AI is telling it to do.

For example, imagine the AI class as the brain of the creature. The brain would not tell the creature to reposition itself at (x,y), instead the brain would tell the creature to move to (x,y). In code,

NO
creature.x = 12;
creature.y = 20;

YES
creature.moveTo(12, 20);

I would use interfaces combined with get/set. In your case it would look like:

class AiCreatureInterface
{
public:
// declare all member variables necessary for AI
virtual int getXCo()=0;
virtual void setYCo(int)=0;
virtual int getYCo()=0;
...
}


class Creature : public AiCreatureInterface
{
..
public:
int getXCo() { return xCo;}
...

}

Your Ai class should look like:

class Ai
{
...
void processAI( AiCreatureInterface& creatureInterface);

}


You could use the same for the path finding class with an other set of declared member variables.

--
Ashaman
Thank you both for the responses. I have been thinking about this code organization problem for most of today, scouring the web, reading all I could on the matter.

I was thinking about something using something like
class Creature : public AiCreatureInterface{.. etc.
I just didn't know how others handle this situation; I wasn't sure if I was on the right track. Now I am thinking you both have good points and I shall head down that path. I am just trying to write and maintain organized and well-commented code since this project is the longest I've written and I need to give myself the best chances of completing it.

Thanks again. Take it easy.
'Controller' type objects such as pather and ai typically shouldnt be separate objects following a strict OOP design. They serve no purpose on their own, and only exist to modify the creature object. Thus, a more efficient layout would have the AI and Pather functions as protected methods of the Creature class. These protected methods would typically be called by some sort of public virtual Update() method. Then, if later on you make a child class of Creature, you can override Update method, which will call the old behavior/ai methods and possibly add some new ones.

The downside of this is you have to put the variables from pather and AI inside your Creature class. It might make Creature a little bigger and harder to understand at a glance. However, this allows you to bypass the issue of giving ai and pather access to xCo/yCo, because theyre in the same class scope by default.
Quote:Original post by WhatsUnderThere
'Controller' type objects such as pather and ai typically shouldnt be separate objects following a strict OOP design. They serve no purpose on their own, and only exist to modify the creature object. Thus, a more efficient layout would have the AI and Pather functions as protected methods of the Creature class. These protected methods would typically be called by some sort of public virtual Update() method. Then, if later on you make a child class of Creature, you can override Update method, which will call the old behavior/ai methods and possibly add some new ones.

I don't think this is good advice. You are violating the single responsibility principle here. Putting the code for pathfinding and AI in separate objects is an excellent idea. Those objects can then later be reused in other places. You should use composition instead of inheritance as much as possible. see this article
Quote:However, this allows you to bypass the issue of giving ai and pather access to xCo/yCo, because theyre in the same class scope by default.

You are not improving the situation here. Throwing all functionality in one class still keeps the dependencies on xCo and yCo you are complaining about. A good design will result in very small interfaces of objects with single responsibilities, without access to another object's internals. Note that I'm not claiming this is easy to do :) The result however, will be worth it.
Thanks for the response, WhatsUnderThere.
Quote:
'Controller' type objects such as pather and ai typically shouldnt be separate objects following a strict OOP design. They serve no purpose on their own, and only exist to modify the creature object.


Yeah, I was finding this to be a common sentiment in the stuff I read today on all these other websites & postings, in a addition to some older GD.net posts.

Anyway, I tried to follow the advice given in this thread and I actually have it refactored and working properly. Nice and commented, nice and easy to understand (for me at least, but this is a solo project =)).

EDIT: Just saw yr post, Koen.

You said,
Quote:Putting the code for pathfinding and AI in separate objects is an excellent idea. Those objects can then later be reused in other places.


Well, I thought so for the path finding, since I do indeed use it in other places.

Thank you for the linked article you gave, Koen. I will have to read it more carefully after I finish this post. I am interested in these differing points of view since I am trying to decide how to best organize my code. Well, off to bed, but please if anyone has anything else to add I will appreciate any other contributions to this discussion.
Quote:Original post by Koen
I don't think this is good advice. You are violating the single responsibility principle here. Putting the code for pathfinding and AI in separate objects is an excellent idea. Those objects can then later be reused in other places. You should use composition instead of inheritance as much as possible. see this article


You are not improving the situation here. Throwing all functionality in one class still keeps the dependencies on xCo and yCo you are complaining about. A good design will result in very small interfaces of objects with single responsibilities, without access to another object's internals. Note that I'm not claiming this is easy to do :) The result however, will be worth it.


I did not suggest merging AI and Creature to decrease dependencies. I was only pointing out that AI, in it's current form, did not have enough responsibility to qualify being its own class. Additionally, delegating movement controls to AI does not simply the interface. Instead, it forces you to create a wide variety of public get() and set() methods that would otherwise be managed through private functions.

While having short source files is a wonderful thing, it's not always the 'OOP' way of doing things. In general, classes should be responsible for modifying themselves. But in this example, the Creature is telling the AI what its state is through get() methods, and then the AI performs some calculations and updates the creature through set() methods. Essentially, the AI has taken away Creature's responsibility of maintaining its own state. If I havent convinced you yet, there really isnt much more to say about it--it's just one of the basic guidelines of OO design.

Also, Im not trying to say to say it's never appropriate to have an AI class. For example, it would make sense to have a separate class if your AI had a complex state consisting of multiple variables, and this state changed over time. Because the AI would have a changing state, you would want the AI class to manage itself rather than put everything in the Creature class. So for every time you called Creature.update(), in that method you would call AI.think() to update the state of the AI. Then, you would poll the AI class for instructions such as the direction to travel in, and update xCo and yCo appropriately. However, the AI should not be modifying the coordinates on its own, it should only be controlling its own state.
Quote:Original post by signal_
What is the best way for the AI class to obtain access to these variables?
Before people get into debating which of the proposed solutions are best, it is crucial to know what these variables are and why the AI class needs them at all. It may also help to know what exactly an AI class is responsible for doing.
Quote:Original post by dmatter
Quote:Original post by signal_
What is the best way for the AI class to obtain access to these variables?
Before people get into debating which of the proposed solutions are best, it is crucial to know what these variables are and why the AI class needs them at all. It may also help to know what exactly an AI class is responsible for doing.


To respond to this, I think it best to first define what the AI class is doing. It has two member functions:

void inputs();     // function to process sensory inputs.void think();      // function for the thinking (control function) to                   // define what state the entity is in.  // the two functions above are composed of other smaller functions.           


So basically, the inputs() function processes the current sensory inputs, while the think() function then calculates what state the entity should be in. It's an AI that is a finite state machine (FSM).

And, the second part of yr question
Quote:what these variables are and why the AI class needs them at all.


These variables are mostly booleans. So far they fall into two classes:

1. self characteristics - like a boolean isRunning, an int hitPoints.
2. characteristics of other perceived objects/environment - like if another creature is perceived, its sex: bool sex, so that a creature could tell if he encountered a female, for example.

So basically, I am implementing a FSM for the AI whose state changes are precipitated by the processing of its internal states by the inputs() method and their analysis in the think() method.

I just wanted to separate the ai code away from the other creature code since the code is getting quite long.

So it seems no matter how it is sliced, I am down to two options. I can use a hierarchical approach or the component based approach....

This topic is closed to new replies.

Advertisement