Re: Class design (again)
Code similar to the following was suggested as a improvement. I have
modified it so it should compile on gcc. In the posters example
acceleration had also been moved from Body to Integrator, in the wider
context of my code I don't think I can do this. Here is the new class
design:
<excerpts follow>
class Integrator {
public:
bool integrate();
void selectIntegrator(IntegratorType t, Body &b);
private:
IntegratorImpl *m_impl;
};
class Body
{
public:
bool integrate() { return m_integrator.integrate(); }
void selectIntegrator( Integrator::IntegratorType type )
{
m_integrator.selectIntegrator( type, *this );
}
};
class IntegratorImpl {
public:
IntegratorImpl(Body &b) : m_body( &b ) {}
virtual bool integrate() = 0;
protected:
Body *m_body;
};
class EulerIntegrator : public IntegratorImpl {
public:
EulerIntegrator(Body &m_body) : IntegratorImpl(m_body) {}
bool integrate() {
// do Euler integration
m_body->velocity_x += m_body->acceleration_x * m_body->dt;
m_body->position_x += m_body->velocity_x * m_body->dt;
// etc...
cout << "Euler integration called" << endl;
return true;
}
};
I believe this is an improvement over my original code but that this
solution would construct lots of largely empty integrator objects,
which only contain a pointer to Body. Is it possible to avoid this?
And also perhaps the overhead caused by the Integrator object? Are
there other solutions I should consider?
I would suggest you change this:
IntegratorImpl(Body &b) : m_body( &b ) {}
virtual bool integrate() = 0;
To
IntegratorImpl() {}
virtual bool integrate(Body& b) = 0;
Right now, your IntegratorImpl has 2 responsibilities:
1) Store Body
2) Implement integration strategy
My suggestion is to get rid of responsibility 1. (OO guideline -
classes should have exactly 1 responsibility. Not always 100% true,
but a good guideline.)
Once you have done that, then you don't need to allocate a new
strategy object each time you set the integration strategy - instead
you can implement your various strategies as Singletons, and just
store a pointer to each in the selectIntegrator method (note: you
would not own that pointer, so you shouldn't delete it). That way, if
you have 5 strategies and 1000 bodies, you would only have 5 strategy
objects, not 1000.
Michael