mtvec
mtvec

Reputation: 18326

Wrapping a library using interfaces without needing downcasts

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?

Upvotes: 19

Views: 1720

Answers (9)

Gnucki
Gnucki

Reputation: 5133

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 language 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.

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');

Upvotes: 6

Tim Lovell-Smith
Tim Lovell-Smith

Reputation: 16135

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.

Upvotes: 7

n. m. could be an AI
n. m. could be an AI

Reputation: 120059

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}; }
};

Upvotes: 5

Dan
Dan

Reputation: 4169

Here's a shot at a solution in Java using generics. I'm not very familiar with C++ so I don't know if a similar solution is possible.

public interface IfaceA<K extends IfaceB> {

  void doSomething(K b);

}


public interface IfaceB {

}


public class ConcreteAForFoo implements IfaceA<ConcreteBForFoo> {

  private FooA fooA;

  @Override
  public void doSomething(ConcreteBForFoo b) {
    fooA.fooBar(b.getFooB());
  }

}


public class ConcreteBForFoo implements IfaceB {

  private FooB fooB;

  public FooB getFooB() {
    return fooB;
  }

}


public class FooA {

  public void fooBar(FooB fooB) {
  }

}


public class FooB {

}

Upvotes: 0

v4n3ck
v4n3ck

Reputation: 51

@Job, I cannot put this in a comment, but it should be there. What techniques have you tried to use in finding a solution to this problem? And why have those techniques not worked? Knowing this would help others explain why a technique might actually work or avoid duplicate effort.

I think the biggest challenge to your problem is that you are expecting an implementation of IfaceA to need to call an as of yet unknown function of an implementation of IfaceB. This will always result in you having to cast the passed in object to get to the method that you want, unless the function that you are calling is in the IfaceB interface.

class IfaceB
{
public:
    virtual void GetB() = 0; // Added function so I no longer have to cast.
}

Now, I would say that your initial implementation is similar to the phrase "If all that you have is a hammer, everything looks like a nail." But, we already know that you have wood screws, metal screws, bolts, lag bolts, etc. So you need to provide a generic way for the "hammer" to not always be a hammer but a drill, wrench, ratchet, ect. That is why I bring up the point of expanding your interface with forcing an implementation of common functions.

Also I noticed in you comments:

I'm not sure if it really matters but it is a library for which the developers should be able to choose which backend to use.

That is actually a huge impact on your design. I see you having 2 things going on here:

  1. Attempting high cohesion, but forcing coupling through allowing implementations to call into each other.
  2. It appears that what you are doing is building a Data Abstraction Layer (DAL).

I have recently implemented a DAL for handling switching out objects that create an image (JPEG, PNG, BMP, etc.). I used an interface for the basic functions, and a factory to handle creating the objects. It has worked out great. I never have to update anything but add a new class to handle a different image type. You can do the same thing with your Iface# implementations by using a Templated Factory. It will allow you to have a class register itself with the factory, and the calling code gets an object based upon a registered name.

Upvotes: 2

Basilevs
Basilevs

Reputation: 23939

Remove doSomethingWith() methods from interfaces. Provide free function:

void doSomethingWith(ConcreteAForFoo & a, ConcreteBForFoo & b);

It is essential to make type represent what it can do and what it can't. Yours can't handle arbitrary subtypes of interfaces.

Upvotes: 0

jxh
jxh

Reputation: 70492

If the goal is to remove the downcast to the concrete implementation from the interface, you will need to expose the concrete type to the interface (but not its implementation). Here is an example:

class LibFoo;
class LibBar;

class IfaceA;
class IfaceB;

template <typename LIB> class BaseA;
template <typename LIB> class BaseB;

struct IfaceA
{
    virtual ~IfaceA () {}
    virtual operator BaseA<LibFoo> * () { return 0; }
    virtual operator BaseA<LibBar> * () { return 0; }
    virtual void doSomethingWith (IfaceB &) = 0;
};

struct IfaceB {
    virtual ~IfaceB () {}
    virtual operator BaseB<LibFoo> * () { return 0; }
    virtual operator BaseB<LibBar> * () { return 0; }
};

The implementations of BaseA and BaseB override the appropriate conversion operator. They also have knowledge of the types it will be interacting with. BaseA relies on the conversion operator for IfaceB to reach the matching BaseB, and then dispatches to the appropriately matched method.

template <typename LIB>
struct BaseA : IfaceA {
    operator BaseA * () override { return this; }

    void doSomethingWith (IfaceB &b) override {
        doSomethingWithB(b);
    }

    void doSomethingWithB (BaseB<LIB> *b) {
        assert(b);
        doSomethingWithB(*b);
    }

    virtual void doSomethingWithB (BaseB<LIB> &b) = 0;
};

template <typename LIB>
struct BaseB : IfaceB {
    operator BaseB * () override { return this; }
};

The concrete implementation can then do what needs to be done.

struct LibFoo {
    class FooA {};
    class FooB {};
};

struct ConcreteFooA : BaseA<LibFoo> {
    void doSomethingWithB (BaseB<LibFoo> &) override {
        //...
    }

    LibFoo::FooA a_;
};

struct ConcreteFooB : BaseB<LibFoo> {
    LibFoo::FooB b_;
};

When adding another library to the mix, a compile time error will result unless the Ifaces are appropriately extended (but the Bases do not necessarily require any extending). You may consider that consequence a useful feature rather than a detrimental one.

struct ConcreteBazA : BaseA<LibBaz> { // X - compilation error without adding
                                      //     conversion operator to `IfaceA`

Working example.


If updating all the interface objects is not an option, then the path of least resistance is to leverage dynamic downcast, since it is designed to resolve exactly this problem.

struct IfaceA
{
    virtual ~IfaceA () {}
    template <typename LIB> operator BaseA<LIB> * () {
        return dynamic_cast<BaseA<LIB> *>(this);
    }
    virtual void doSomethingWith (IfaceB &) = 0;
};

struct IfaceB {
    virtual ~IfaceB () {}
    template <typename LIB> operator BaseB<LIB> * () {
        return dynamic_cast<BaseB<LIB> *>(this);
    }
};

Since the conversion is no longer virtual, the BaseA and BaseB templates no longer need the override conversion method.

template <typename LIB>
struct BaseA : IfaceA {    
    void doSomethingWith (IfaceB &b) override {
        doSomethingWithB(b);
    }

    void doSomethingWithB (BaseB<LIB> *b) {
        assert(b);
        doSomethingWithB(*b);
    }

    virtual void doSomethingWithB (BaseB<LIB> &b) = 0;
};

template <typename LIB>
struct BaseB : IfaceB {
};

This answer can be considered an illustration of the visitor pattern suggested by n.m.

Upvotes: 3

Chris
Chris

Reputation: 2763

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();

Upvotes: 3

Basilevs
Basilevs

Reputation: 23939

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).

Upvotes: 7

Related Questions