How to create a public API for an existing codebase?

Question

I'm trying to create a public C++ API for a private code base that is also written in C++. I tried to do this as follows:

• Create a wrapper API class for each private class.
• Have a common API base class that has a pointer to the private class.
• The API class implementations don't contain any functionality but simply forward the method calls to their private counterparts.

This seemed like a reasonable approach. However, in reality it quickly results in very clumsy code. Here is an example that represents the problem I'm experiencing.

(Edit: the code can also be viewed here).

The private API classes (no problem here)

namespace Core {


// Base class for all Core classes.
class CoreObject
{
public:
    virtual ~CoreObject();
};    

class Interface;
class Stream;    

class Server : public CoreObject
{
public:
    Interface * createInterface();

private:
    std::vector<Interface*> mInterfaces;

};    

class Interface : public CoreObject
{
public:
    void addStream(Stream * stream);

    const Stream * getStreamByIndex(std::size_t index) const;

    std::size_t streamCount() const;

private:
    std::vector<Stream*> mStreams;
};    

class Stream : public CoreObject
{
public:
    void start();

    void stop();

private:
    std::string mStats;
};


} // Core

The public API class declarations (so far so good)

namespace Core {
class CoreObject;
}


namespace API {


class APIStream;
class APIInterface;    

// Base class for all API classes.
class APIObject
{
public:
    APIObject(Core::CoreObject * inCoreObject);

    virtual ~APIObject();

    Core::CoreObject * getCoreObject();

    const Core::CoreObject * getCoreObject() const;

    void setCoreObject(Core::CoreObject * inCoreObject);

private:
    Core::CoreObject * mCoreObject; 
};    

class APIServer : public APIObject
{
public:
    APIServer();

    APIInterface * createInterface();
};    

class APIInterface : public APIObject
{
public:
    APIInterface();

    void addStream(APIStream * stream);

    const APIStream * getStreamByIndex(std::size_t index) const;

    APIStream * getStreamByIndex(std::size_t index);

    std::size_t streamCount() const;
};    

class APIStream : public APIObject
{
public:
    APIStream();

    void start();

    void stop();
};

The API implementation (too many casts, generally clumsy)

#include "API.h"
#include "Core.h"


namespace API {


APIObject::APIObject(Core::CoreObject * inCoreObject) :
    mCoreObject(inCoreObject)
{
}

APIObject::~APIObject()
{
}

Core::CoreObject * APIObject::getCoreObject()
{
    return mCoreObject;
}


const Core::CoreObject * APIObject::getCoreObject() const
{
    return mCoreObject;
}


void APIObject::setCoreObject(Core::CoreObject * inCoreObject)
{
    mCoreObject = inCoreObject;
}


//
// APIServer
//
APIServer::APIServer() :
    APIObject(new Core::Server)
{
}    

APIInterface * APIServer::createInterface()
{
    Core::Server * coreServer = static_cast<Core::Server*>(getCoreObject());
    Core::Interface * coreInterface = coreServer->createInterface();
    APIInterface * result(new API::APIInterface);
    result->setCoreObject(coreInterface);
    return result;
}


//
// APIInterface
//
APIInterface::APIInterface() :
    APIObject(new Core::Interface)
{
}    

void APIInterface::addStream(APIStream * apiStream)
{
    Core::Stream * coreStream = static_cast<Core::Stream *>(apiStream->getCoreObject());
    Core::Interface * coreInterface = static_cast<Core::Interface*>(getCoreObject());
    coreInterface->addStream(coreStream);
}


//
// APIStream
//
const APIStream * APIInterface::getStreamByIndex(std::size_t index) const
{
    const Core::Interface * coreInterface = static_cast<const Core::Interface*>(getCoreObject());
    const Core::Stream * coreStream = coreInterface->getStreamByIndex(index);

    // Now how I get the the APIStream object?
    return 0;
}

std::size_t APIInterface::streamCount() const
{
    const Core::Interface * coreInterface = static_cast<const Core::Interface*>(getCoreObject());
    return coreInterface->streamCount();
}    

APIStream::APIStream() :
    APIObject(new Core::Stream)
{
}    

void APIStream::start()
{
    static_cast<Core::Stream*>(getCoreObject())->start();
}       

void APIStream::stop()
{
    static_cast<Core::Stream*>(getCoreObject())->stop();
}    

} // API

As you can see the implementation looks doesn't look too good. I would appreciate your answers or insights regarding these questions:

• Where did I go wrong?
• What should I have done instead?

Update

John Dibling's suggestion seems to work really well. As you can see in the improved code, all problems are solved neatly.

I still have to apply this solution to the real code at work tomorrow. I'm interested to see how well it works there.

Solution 1

When doing this kind of thing, where there is more or less a one-to-one relation between the API and implementation objects, I'll generally use a static factory method, and have the implementation class derive from the API class. Like this:

file: api.h

class Interface
{
public:
  static Interface* Create();
  virtual void Foo() = 0;
};

file: impl.h

class Concrete : public Interface
{
public:
  void Foo() {};
};

file: impl.cpp

Interface* Interface::Create()
{
  return new Concrete;
}

This has a number of benefits. Including:

1. Create can construct many different implementations, and Interface's ABI doesn't have to change.
2. The return types rely on the language's covariance rules, so no casts are needed.
3. Debugging is somewhat easier, because you can tell exactly what kind of Interface you've got.

Solution 2

See Adapter Pattern

Comments

[StackedCrooked]

This seems like a good idea. Especially (2) is a good point. I usually follow the "prefer composition over inheritance" rule for separating interface and implementation, but this seems like situation where inheritance has the advantage.

[StackedCrooked]

I think the whole setup with the API classes is an (unsuccessful) application of the Adapter pattern.

[abhimanyuaryan]

how can impl.h header file have definition of Foo()? Header files don't contain definitions? Right?