mardi 25 août 2015

Thread safety with decorator pattern

This is rather long but I will try to be as clear as I can.

I have an interface, lets call it IFoo

class IFoo
{
public:
    virtual void reset(const Bar* bar) = 0;
    virtual int calculate(int i) const = 0;
};

a basic implementation

class FooBasic : public IFoo
{
private:
    int _value;
public:
    FooBasic(int value) : _value(value) {}

    virtual void reset(const Bar* bar) override {}
    virtual int calculate(int i) const override { return _value; }
};

and I have a base decorator for the interface

class FooDecorator : public IFoo
{
protected:
    IFoo* _foo;
public:
    FooDecorator(IFoo* foo) : _foo(foo) {}

    virtual void reset(const Bar* bar) override { _foo->reset(bar); }
    virtual int calculate(int i) const override { return _foo->calculate(i); }
};

and I have a tonne of implementations of the decorator which take pretty much the same form

class FooInt : public FooDecorator
{
private:
    int _y;
    int _z;
public:
    FooInt(IFoo* foo) : FooDecorator(foo), _y(0), _z(0) {}

    virtual void reset(const Bar* bar) override
    {
        _foo->reset(bar);
        _y = bar->expensiveIntFunction(15);
        _z = bar->expensiveIntFunction(10);
    }
    virtual int calculate(int i) const override { return _y*_foo->calculate(i) + _z; }
};

class FooIntStar : public FooDecorator
{
private:
    const int* _z;
public:
    FooIntStar(IFoo* foo) : FooDecorator(foo), _z(nullptr) {}

    virtual void reset(const Bar* bar) override
    {
        _foo->reset(bar);
        _z = bar->expensiveIntStarFunction();
    }
    virtual int calculate(int i) const override { return _foo->calculate(i)*_z[i]; }
};

The idea here is that clients of the IFoo interface will first call the reset() method passing in their Bar object. Then they will call the calculate() method millions of times with different integer parameters. The IFoo implementations will on the call to reset() extract some information from the Bar object and use it to set some internal members which they will then use to decorate the result from the underlying IFoo object. What they extract from the bar object depends on the implementation and could be different types/values.

This all works OK as long as there is a single client using the IFoo implementation. I am now trying to introduce some threading where I have different clients using the IFoo implementations concurrently. Each client has its own Bar object but I would like them to use the same instances of the IFoo objects (if possible). In the current state this will not work because the call to reset() sets member variables i.e. the reset() method is not const.

My plan it to create a workspace object which then gets passed around. Each client will own its own instance of the workspace to avoid conflict when several clients use the same object. On the call to reset, the IFoo implementations will set the temporary data in the workspace and not use internal members. However, since the type of data required for the different implementations is different it is tricky designing this workspace in advance.

One idea I have come up with is to make this workspace abstract and have the implementations extend it. So I'm thinking of changing the interface to

// Totally undefined object
struct IFooWorkspace
{
    virtual ~IFooWorkspace(){}
};

class IFoo
{
public:
    virtual IFooWorkspace* createWorkspace() const = 0;
    virtual void reset(IFooWorkspace* workspace, const Bar* bar) const = 0;
    virtual int calculate(IFooWorkspace* workspace, int i) const = 0;
};

and for instance the FooInt implementation

class FooInt : public FooDecorator
{
private:
    // Struct definition instead of member variables
    struct WorkSpace : IFooWorkspace
    {
        WorkSpace() : _y(0), _z(0), _ws(nullptr) {}
        virtual ~WorkSpace(){ delete _ws; }
        int _y;
        int _z;
        IFooWorkspace* _ws;
    };
public:
    FooInt(IFoo* foo) : FooDecorator(foo), _y(0), _z(0) {}

    virtual IFooWorkspace* createWorkspace() const override
    {
        WorkSpace* ws = new WorkSpace;
        ws->_ws = _foo->createWorkspace();
        return ws;
    }

    virtual void reset(IFooWorkspace* workspace, const Bar* bar) const override
    {
        WorkSpace& ws = dynamic_cast<WorkSpace&>(*workspace);
        ws._y = bar->expensiveIntFunction(15);
        ws._z = bar->expensiveIntFunction(10);
        _foo->reset(ws._ws, bar);
    }

    virtual int calculate(IFooWorkspace* workspace, int i) const override
    {
        const WorkSpace& ws = dynamic_cast<WorkSpace&>(*workspace);
        return ws._y*_foo->calculate(ws._ws, i) + ws._z;
    }
};

So the new plan is that clients will call createWorkspace() first and own the IFooWorkspace object returned. Then subsequent calls to reset() and calculate() should pass this workspace.

My problem is that it all looks rather convoluted and I'm not sure about the dynamic_cast in terms of safety and performance. So the question is: is there a simpler, cleaner way to achieve thread safety here? Ideally I would like to avoid creating new instances for the IFoo objects.

Aucun commentaire:

Enregistrer un commentaire