Search code examples
c++templatesinheritancerefactoringclass-design

Refactoring using Abstract Base Classes and Templates in C++


I have a problem with some refactoring I am attempting. We have a lot of code duplication and I am trying to sort this out. I have the following class structure

IMessageSink.h:

class IMessageSink
{
public:
    virtual ~IMessageSink() { };
    virtual void process(const taurus::Msg& msg) = 0;
};

I have the following base class ModelBase.h which all models must inherit from, at this point please not the use of the friend class EM:

class ModelBase : public virtual IMessageSink
{
public:
   ModelBase(Tag a);

   void process(const taurus::Msg& msg);
   void reset();

private:
   friend class EM; // I will ask about this below.

   virtual void calculate(double lambda) = 0;
};

The implementation of friend EM is not correct and I ask about this below. I then have a class that implements/inherits from ModelBase, ModelM0.h:

class ModelM0 : public virtual ModelBase
{
public:
    ModelM0(Tag a);

   static ModelM0* ModelM0::make(Tag a)
   {
      ModelM0* m = new ModelM0(a);
      m->reset();
      return m;
   }

private:
    void calculate(double lambda);
};

with ModelM0.cpp implemented as:

ModelM0::ModelM0(Tag a) : ModelBase(a) { }

void ModelM0::calculate(double lambda)
{
    // Do stuff.
}

The problem is with the EM friend class and how to implement this in a generic fashion. Previously, this class only worked with types ModelM0 which did not inherit from ModelBase. Now other models also inherit from ModelBase and EM needs to work with these as well - here lies the problem. I have the following definition in EM.h (which I have changed to a template so we can specify the type of ModelBase we are using TModel):

with EM.h as:

template <class TModel> 
class EM : public virtual IMessageSink
{
public:
    static EM* make(Tag a)
    {
        return new EM(a);
    }

    EM(Tag a);
    ~EM();

    void process(const taurus::Msg& msg);
    void run();
private:
    struct Bucket
    {
        TModel* _model;
        std::vector<TinyMatrix<1, 1> > _obs
    };

    EM::Bucket& getModel(int ag);
}

The problem implementation is EM::Bucket& getModel(int ag);, in EM.cpp we have

template<class TModel> 
EM<TModel>::EM(Tag a) { }

template<class TModel> 
EM<TModel>::~EM()
{
    run();
}

template<class TModel>
void EM<TModel>::process(const taurus::Msg& msg)
{
    int ag = getMessageCount(msg.type()); // External call.
    if (ag <= 3)
    {
        Bucket& b = getModel(ag);
        TModel* m = b._model;
        m->process(msg);
    }
}

The above seems to be okay, my problem is the implementation of getModel

template<class TModel> 
EM<TModel>::Bucket& EM<TModel>::getModel(int ag)
{
    // This is not right.
    TModel* m;
    m = TModel::make(getTag(ag)); // This is not right - I need a factory.

    // ... Do stuff.

    Bucket& b = // Get a bucket.
    b._model = m;

    return b;
}

My questions:

  1. How can I change the above code so that in EM<TModel>::getModel(int ag) I can create the correct TModel using make in the above - do I need a factory and how would this be implemented?

  2. In ModelBase.h the EM class is specified as a friend class. How can this me made generic to work with the TModel (ModelBase) type being used?

It is important to note here that this is a refactoring question, not whether-or-not the code I have shown in the methods is proper or correct (this has been cut down in order to succinctly highlight my problems). The refactoring is the only thing I would like help with. Thanks very much for your time.


Solution

  • When I tried to get your code compiling, I had to fix a few missing semicolons and missing types (Tag, taurus::Msg, TinyMatrix) and also fix the declaration and definition of getModel(int ag)

    Generally, you need to indicate to the compiler, that Bucket is actually a type name and not some other sort of parameter.

    For the declaration you have 2 options:

    Bucket& getModel(int ag); // (1)
    typename EM<TModel>::Bucket& getModel(int ag); // (2)
    

    (1) is implicit usage of the Bucket type of your current template specialization. (2) is explicit type usage together with the typename keyword for the compiler, as mentioned above.

    For the definition, you definitely need to typename keyword, since you are out of your class definition context.

    template<class TModel>
    typename EM<TModel>::Bucket& EM<TModel>::getModel(int ag)
    {
        // This is not right.
        TModel* m;
        m = TModel::make(getTag(ag)); // This is not right - I need a factory.
    
        // ... Do stuff.
    
        Bucket& b = // Get a bucket.
            b._model = m;
    
        return b;
    }
    

    Just ignore the "This is not right." comments - I copied them over from your sample code. Its actually perfectly right.

    For the friend declaration, you need to add a template version, since you want to befriend all possible template instantiations. I looked it up from this answer (Credits to Anycorn)

    template <class> friend class EM;
    

    Hope that solves all your issues. Note I used template <class> since you used it. Personally I prefer template <typename>.