Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

Wrapping a library using interfaces without needing downcasts

Tags:

c++

oop

interface

Let's say my project uses a library, LibFoo, which provides its functionality through a number of classes, say, FooA and FooB. Now, there are a number of similar libraries (e.g., LibBar which provides BarA and BarB) that provide the same functionality as LibFoo and I want users of my project to be able to chose which library to use, preferably at runtime.

For this to work, I've created a "wrapper layer" that defines the interfaces I expect from the libraries. In my example, this layer contains two interfaces: IfaceA and IfaceB. Then, for every library I want to support, I create an "implementation layer" that implements the interfaces using one of the libraries.

My problem now lies in how to implement the implementation layer nicely. To demonstrate my problem, consider we have the following interfaces (shown in C++ but should be applicable to similar languages):

class IfaceA
{
public:
    virtual void doSomethingWith(IfaceB& b) = 0;
    ...
};

class IfaceB
{
    ...
};

The classes in the implementation layer of LibFoo will hold objects from the corresponding classes of LibFoo. The operations in the interfaces should be implemented using these objects. Hence (excuse me for the horrible names):

class ConcreteAForFoo : public IfaceA
{
public:
    void doSomethingWith(IfaceB& b) override
    {
        // This should be implemented using FooA::doSomethingWith(FooB&)
    }

private:
    FooA a;
};

class ConcreteBForFoo : public IfaceB
{
public:
    FooB& getFooB() const
    {
        return b;
    }

private:
    FooB b;
};

The problem is in implementing ConcreteAForFoo::doSomethingWith: its parameter has type IfaceB& but I need access to the implementation details of ConcreteBForFoo to be able to implement the method correctly. The only way I've found to do this is to use ugly downcasts all over the place:

void doSomethingWith(IfaceB& b) override
{
    assert(dynamic_cast<ConcreteBForFoo*>(&b) != nullptr);
    a.doSomethingWith(static_cast<ConcreteBForFoo&>(b).getFooB());
}

Since having to downcast is generally considered to be a code smell, I can't help to think there should be a better way to do this. Or am I designing this wrongly to begin with?

TL;DR

Given a layer of interdependent interfaces (in that methods in one interface receive references to other interfaces). How can the implementations of these interfaces share implementation details without downcasting or exposing those details in the interfaces?

like image 663
mtvec Avatar asked Dec 01 '14 08:12

mtvec


5 Answers

Looks like a classic double dispatch usecase.

class IfaceA
{
public:
    virtual void doSomethingWith(IfaceB& b) = 0;
    ...
};

class IfaceB
{
    virtual void doSomethingWith(IfaceA & a) = 0;
    ...
};

struct ConcreteAForFoo : public IfaceA {
    virtual void doSomethingWith (IfaceB &b) override {
        b.doSomethingWith(*this);
    }
};

struct ConcreteBForFoo : public IfaceB
{
    virtual void doSomethingWith(IfaceA & a) override {
        //b field is accessible here
    }

private:
    FooB b;
};

IfaceB::doSomethingWith signature can be varied to obtain optimal balance between coupling and access level (For example, ConcreteAForFoo can be used as argument to get access to its internals at cost of tight coupling).

like image 190
Basilevs Avatar answered Nov 09 '22 15:11

Basilevs


The correct answer is either:

  1. Make your interfaces truly interoperable and interchangeable, or
  2. Don't change anything. Keep doing dynamic casts.
  3. Add something else to your type system which makes it impossible for user to do the wrong thing and mix incompatible concrete implementations.

Your basic problem right now is that the interface/inheritance is an 'oversimplification lie' by which I really mean your interfaces aren't actually following LSP.

If you want to fix this follow LSP and make the libraries mixable, you need to do 1: fix your code to not use sneaky implementation details, and really follow LSP. But this option is basically precluded by the question statement, where it is explicit that the implementation of the classes is incompatible, and we should probably assume this will always be the case.

Assuming the libraries are never going to be mixable, the correct answer is either 2. keep using dynamic casts. Let's think about why:

OP's interface definition literally says that ConcreteAForFoo can successfully doSomethingWith any IFaceB object. But OP knows that isn't true - it really has to be a ConcreteBForFoo because of a need to use certain implementation details which won't be found in interface of IFaceB.

Downcasting is the best thing to do in this particular described scenario.

Remember: downcasting is for when you know the type of the object, but the compiler doesn't. The compiler only knows the method signature. You know the runtime types better than the compiler does because you know only one library is loaded at a time.

You want to hide the true type from the compiler because you want to abstract your libraries into an interface and hide underlying types from the users of your library, which I think is a fine design decision. Downcasting is the implementation part of your abstraction where you tell the compiler 'trust me, I know this is going to work, that was just an abstraction'.

(And using dynamic_cast instead of static_cast says 'and oh yes, because I'm a little paranoid, please let the runtime please throw an error if I ever happen to be wrong about this', e.g. if someone misuses by library by trying to mix incompatible ConcreteImplementations.)

Option 3 is thrown in there as even though I think it's not what OP really wants, since it means breaking the 'interface compatibility' constraint, it's another way to stop violating LSP, and satisfy fans of strong typing.

like image 21
Tim Lovell-Smith Avatar answered Nov 09 '22 14:11

Tim Lovell-Smith


First of all, I would like to thank you because your question is nicely asked.

My first feeling is that you have an architecture problem if you want to pass a IfaceB& but need to use a concrete type.

However, I understand the complexity of what you want to do so I will try to give you a nicer solution than the downcast.

The beginning of your architecture is a good choice because this is an adapter pattern (the link is in C# but even if I don't use this langage for a while it's still the best I found on the subject!). And this is exactly what you want to do.

In the following solution, you add another object c responsible of the interaction between a and b:

class ConcreteAForFoo : public IfaceA
{
    private:
        FooA a;
};

class ConcreteBForFoo : public IfaceB
{
    private:
        FooB b;
};

class ConcreteCForFoo : public IfaceC
{
    private:
        ConcreteAForFoo &a;
        ConcreteBForFoo &b;

    public:
        ConcreteCForFoo(ConcreteAForFoo &a, ConcreteBForFoo &b) : a(a), b(b)
        {
        }

        void doSomething() override
        {
            a.doSomethingWith(b);
        }
};

class IfaceC
{
    public:
        virtual void doSomething() = 0;
};

You should use a factory to get your objects:

class IFactory
{
    public:
        virtual IfaceA* getA() = 0;
        virtual IfaceB* getB() = 0;
        virtual IfaceC* getC() = 0;
};

class IFactory
{
    public:
        virtual IfaceA* getA() = 0;
        virtual IfaceB* getB() = 0;
        virtual IfaceC* getC() = 0;
};

class FooFactory : public IFactory
{
    private:
        IfaceA* a;
        IfaceB* b;
        IfaceC* c;

    public:
        IfaceA* getA()
        {
            if (!a) a = new ConcreteAForFoo();

            return a;
        }

        IfaceB* getB()
        {
            if (!b) b = new ConcreteBForFoo();

            return b;
        }

        IfaceC* getC()
        {
            if (!c)
            {
                c = new ConcreteCForFoo(getA(), getB());
            }

            return c;
        }
};

Of course, you will certainly have to adapt this code because you may have many b or a.

At this point, you will be able to process the method doSomething like that:

factory = FooFactory();
c = factory.getC();

c->doSomething();

There is maybe a better solution but I would need a real code to tell you that. I hope this will help you.

Finally, I would like to apologize for my C++ (and my english), it's a long long time I didn't code in C++ (and I know I did at least a few mistakes (what means null == this.a without a pointer??)).

EDIT:

Another possibility to avoid the ambiguity of offering the possibility to pass an interface whereas you want a concrete type is to use a kind of mediator (responsible of the interaction between instances of A and B) associated with a named registry:

class FooFactory : public IFactory
{
    private:
        IMediator* mediator;

    public:
        IfaceA* getNewA(name)
        {
            a = new ConcreteAForFoo();
            mediator = getMediator();
            mediator->registerA(name, *a);

            return a;
        }

        IfaceB* getNewB(name)
        {
            b = new ConcreteBForFoo();
            mediator = getMediator();
            mediator->registerB(name, *b);

            return b;
        }

        IMediator* getMediator()
        {
            if (!mediator) mediator = new ConcreteMediatorForFoo();

            return mediator;
        }
};

class ConcreteMediatorForFoo : public IMediator
{
    private:
        std::map<std::string, ConcreteAForFoo> aList;
        std::map<std::string, ConcreteBForFoo> bList;

    public:
        void registerA(const std::string& name, IfaceA& a)
        {
            aList.insert(std::make_pair(name, a));
        }

        void registerB(const std::string& name, IfaceB& b)
        {
            bList.insert(std::make_pair(name, b));
        }

        void doSomething(const std::string& aName, const std::string& bName)
        {
            a = aList.at(aName);
            b = bList.at(bName);

            // ...
        }
}

You can then handle the interaction of instances of A and B like that:

factory = FooFactory();
mediator = factory.getMediator();
a = factory.getNewA('plop');
bPlap = factory.getNewB('plap');
bPlip = factory.getNewB('plip');
// initialize a, bPlap and bPlip.

mediator->doSomething('plop', 'plip');
like image 25
Gnucki Avatar answered Nov 09 '22 14:11

Gnucki


This is not an easy task. The type system off C++ is not quite adequate. Nothing in principle can (statically) prevent your users to instantiate IFaceA from one library and IFaceB from the other library, and then mix and match them as they see fit. Your options are:

  1. Don't make the libraries dynamically selectable, i.e. don't make them implement same interfaces. Instead, let them implement instances of a family of interfaces.

    template <typename tag>
    class IfaceA;
    template <typename tag>
    class IfaceB;
    
    template <typename tag>
    class IfaceA
    {
       virtual void doSomethingWith(IfaceB<tag>& b) = 0;
    };
    

    Each library implements interfaces with a different tag. Tags can be easily selectable by the user at compile time, but not at run time.

  2. Make interfaces really interchangeable, so that users can mix and match interfaces implemented by different libraries.
  3. Hide the casts behind some nice interface.
  4. Use the visitor pattern (double dispatch). It eliminates the casts, but there's a well known cyclic dependency problem. The acyclic visitor eliminates the problem by introducing some dynamic casts, so this is a variant of #3.

Here is a basic example of double dispatch:

//=================

class IFaceBDispatcher;
class IFaceB 
{
   IFaceBDispatcher* get() = 0;
};

class IfaceA
{
public:
    virtual void doSomethingWith(IfaceB& b) = 0;
    ...
};

// IFaceBDispatcher is incomplete up until this point

//=================================================

// IFaceBDispatcher is defined in these modules

class IFaceBDispatcher
{
  virtual void DispatchWithConcreteAForFoo(ConcreteAForFoo*) { throw("unimplemented"); }
  virtual void DispatchWithConcreteAForBar(ConcreteAForBar*) { throw("unimplemented"); }

};
class ConcreteAForFoo : public IfaceA
{
   virtual void doSomethingWith(IfaceB& b) { b.DispatchWithConcreteAForFoo(this); }
}

class IFaceBDispatcherForFoo : public IFaceBDispatcher
{
   ConcreteBForFoo* b;
   void DispatchWithConcreteAForFoo(ConcreteAForFoo* a) { a->doSomethingWith(*b); }
};   

class IFaceBDispatcherForBar : public IFaceBDispatcher
{
   ConcreteBForBar* b;
   void DispatchWithConcreteAForBar(ConcreteAForBar* a) { a->doSomethingWith(*b); }
};   

class ConcreteBForFoo : public IFaceB 
{
   IFaceBDispatcher* get() { return new IFaceBDispatcherForFoo{this}; }
};

class ConcreteBForBar : public IFaceB 
{
   IFaceBDispatcher* get() { return new IFaceBDispatcherForBar{this}; }
};
like image 5
2 revs Avatar answered Nov 09 '22 14:11

2 revs


In your design it's possible to instantiate both libFoo and libBar at the same time. It is also possible to pass IFaceB from libBar to IFaceA::doSomethingWith() of libFoo.

As a result you are forced to dynamic_cast down to ensure that a libFoo object is not passed to libBar object and visaversa. Need to validate that the user did not mess up.

I only see two things you can do really:

  • accept the dynamic_cast to validate you interface on a per-function basics
  • redesign the interface so that none of the function parameters are interfaces

You may be able to accomplish the second task only allowing objects to be created from inside other objects and each created object maintains reference to the object which created it.

So for example for libFoo:

IFaceA* libFooFactory (void)
{
    return new libFoo_IFaceA ();
}

IFaceB* libFoo_IFaceA::CreateIFaceB (void)
{
    return new libFoo_IFaceB (this);
}

.
.
.

libFoo_IFaceB::libFoo_IFaceB (IFaceA* owner)
{
    m_pOwner = owner;
}

libFoo_IFaceB::doSomething (void)
{
    // Now you can guarentee that you have libFoo_IFaceA and libFoo_IFaceB
    m_pOwner-> ... // libFoo_IFaceA
}

Usage make look like:

IFaceA* libFooA = libFooFactory ();
IFaceB* libFooB = libFooA->CreateIFaceB();

libFooB->doSomething();
like image 3
Chris Avatar answered Nov 09 '22 16:11

Chris