views:

72

answers:

3

In my game engine I have a State class that represents the game world at a point in time. This State contains a number of Body objects that define one rigid body each. Each State has a container to keep track of the Body objects it owns, and each Body has a pointer back to its parent State.

Outline of State class:

class State {
private:
  std::set<Body*> _bodies;

public:
  //It could be done here
  void addBody(Body* body) {
    //Remove from old State
    if (body->_state)
      body->_state->_bodies.erase(this);

    //Set Body's back-reference
    body->_state = this;

    //Add to this State
    _bodies.insert(body);
  }
};

Outline of Body class:

class Body {
private:
  State* _state;

public:
  //It could also be done here
  void setState(State* state) {
    //Remove from old State
    if (_state)
      _state->_bodies.erase(this);

    //Set back-reference
    _state = state;

    //Add to the new State
    if (_state)
      _state->bodies.insert(this);
  }
};

The problem, if there is one, is that adding/removing a Body to/from a State requires changing a private member of each.

At one point I considered having a State-Body-mediator static class, which would have friend access to both classes. It sounds nice, because the State-Body relationship would be explicitly managed by that name, but it's a lot of declaration overhead for managing a simple relationship.

Is there a "better" way to do this? I'd like to see what kinds of ideas are out there.

+1  A: 

Well, one way you could do it would be to have a friend function of both classes:

void associate(State& s, Body& b)
{
    s.addBody(&b);
    b.setState(&s);
}
rlbond
This is true, there's no reason that each relationship management function would have to be in a separate class.
mvanbem
A: 

At a first pass, I'd combine the two approaches:

void State::addBody(Body* body) {
    //Add to this State
    bodies_.insert(body); 

    //Remove from old State
    body->setState(this);
}

void State::removeBody(Body* body) {
    bodies_.erase(body); 
}


void Body::setState(State* state) {
    if(state_) {
        state_->removeBody(this);
    }
    state_ = state;
}

This decouples the internals of State and Body, so you can e.g. use some completely different mechanism for State's Body list and Body need never know about it. You'll want to be careful about exception safety; this example intentionally puts the container insertion at the beginning, so if that throws, the objects' state won't be changed, but I don't know for sure that nothing else in here can throw.

Note that I moved the underscores to the ends of the member variables; I know leading underscores is a common convention, but they're actually reserved to the compiler by the C++ standard.

ceo
Well, actually leading underscores are OK as long as they're not followed by a capital letter. However I like the trailing underscore better for my own aesthetic reasons.
rlbond
A: 

The purpose of private is to have one class which is responsible for maintaining the body-state linkage. The practical effect is that when the linkage goes wrong, or needs to be enhanced in some way, you have only one class to inspect or modify. You could use friend to split the responsibility, but you may create headaches for yourself in the long-term. I would keep one or both classes 'dumb'.

// example 1: dumb state
void State::addBody(Body* body) { _bodies.insert(body); }
void State::removeBody(Body* body) { _bodies.erase(body); }
void Body::setState(State* state)
{
    if(_state) _state->removeBody(this);
    if(state) state->addBody(this);
    _state = state;
}

// example 2: dumb body
void Body::setState(State* state) { _state = state; }
State* Body::getState() const { return _state; }
void State::addBody(Body* body)
{
    if(body && body->getState() != this)
    {
        body->getState()->_bodies.erase(body);
        _bodies.insert(body);
        body->setState(this);
    }
}

// example 3: both are dumb
void State::addBody(Body* body) { _bodies.insert(body); }
void State::removeBody(Body* body) { _bodies.erase(body); }
void Body::setState(State* state) { _state = state; }
State* Body::getState() const { return _state; }
void BodyStateMachine::setBodyState(Body* body, State* state)
{
    if(body && body->getState() != state)
    {
        body->getState()->removeBody(body);
        state->addBody(body);
        body->setState(state);
    }
}

The method you choose should depend upon which class will be more publicly used (you would prefer that other code manages the "smart" class). You could protect the dumb class(es) further by creating an interface that does not expose the getter/setter functions.

Evan Rogers