C++ API design: Clearing up public interface

Question

For my library, I want to expose a clean public API that does not distract with implementation details. As you have it, though, these details are leaking even to the public realm: Some classes have valid public methods that are used by the rest of the library, but aren't very useful for the user of the API and as such don't need to be a part of it. A simplified example of the public code:

class Cookie;

class CookieJar {
public:
    Cookie getCookie();
}

class CookieMonster {
public:
    void feed(CookieJar cookieJar) {
        while (isHungry()) {
            cookieJar.getCookie();
        }
    }

    bool isHungry();
}

The getCookie() method of a CookieJar is not useful to the user of the library, who presumably does not like cookies anyway. It is, however, used by the CookieMonster to feed itself, when given one.

There are some idioms that help solve this issue. The Pimpl idiom offers to hide the private members of a class, but does little to disguise the public methods that are not supposed to be a part of the API. It is possible to move those into the implementation class as well, but you would then need to provide direct access to it for the rest of the library to use. Such a header would look like this:

class Cookie;
class CookieJarImpl;

class CookieJar {
public:
    CookieJarImpl* getImplementation() {
        return pimpl.get();
    }
private:
    std::unique_ptr<CookieJarImpl> pimpl;
}

It is handy if you really need to prevent user access to these methods, but if it's merely an annoyance, this doesn't help very much. In fact, new the method is now even more useless than the last, because the user does not have access to the implementation of CookieJarImpl.

An alternative approach is to define the interface as an abstract base class. This gives explicit control over what is a part of the public API. Any private details can be included in the implementation of this interface, which is inaccessible to the user. The caveat is that the resulting virtual calls impact performance, even more so than the Pimpl idiom. Trading speed for a cleaner API is not very attractive for what is supposed to be a high performance library.

To be exhaustive, yet another option is to make the problematic methods private and use friend classes where needed to access them from the outside. This, however, gives the target objects access to the truly private members as well, somewhat breaking encapsulation.

So far the best solution to me seems to be the Python way: Instead of trying to hide the implementation details, just name them appropriately, so that they are easily identifiable as not part of the public API and do not distract from regular usage. The naming convention that comes to mind is using an underscore prefix, but apparently such names are reserved for the compiler and their use is discouraged.

Are there any other c++ naming conventions for distinguishing members that are not intended to be used from outside the library? Or would you instead suggest me to use one of the alternatives above or something else I missed?

Answer 1

Answering my own question: This idea is based on the interface - implementation relationship, where the public API is explicitly defined as the interface, while the implementation details reside in a separate class extending it, inaccessible to the user, but accessible to the rest of the library.

Halfway through implementing static polymorphism using CRTP as πάντα ῥεῖ suggested to avoid virtual call overhead, I realized polymorphism is not actually needed at all for this kind of design, as long as only one type will ever implement the interface. That makes any kind of dynamic dispatch pointless. In practice, this means flattening all the ugly templates you get from static polymorphism and ending up with something very simple. No friends, no templates, (almost) no virtual calls. Let's apply it to the example above:

Here is the header, containing just the public API with example usage:

class CookieJar {
public:
    static std::unique_ptr<CookieJar> Create(unsigned capacity);

    bool isEmpty();
    void fill();

    virtual ~CookieJar() = 0 {};
};

class CookieMonster {
public:
    void feed(CookieJar* cookieJar);
    bool isHungry();
};

void main() {
    std::unique_ptr<CookieJar> jar = CookieJar::Create(20);
    jar->fill();
    CookieMonster monster;
    monster.feed(jar.get());
}

The only change here is turning CookieJar into an abstract class and using a factory pattern instead of a constructor.

The implementations:

struct Cookie {
    const bool isYummy = true;
};

class CookieJarImpl : public CookieJar {
public:
    CookieJarImpl(unsigned capacity) :
        capacity(capacity) {}

    bool isEmpty() {
        return count == 0;
    }

    void fill() {
        count = capacity;
    }

    Cookie getCookie() {
        if (!isEmpty()) {
            count--;
            return Cookie();
        } else {
            throw std::exception("Where did all the cookies go?");
        }
    }

private:
    const unsigned capacity;
    unsigned count = 0;
};

// CookieJar implementation - simple wrapper functions replacing dynamic dispatch
std::unique_ptr<CookieJar> CookieJar::Create(unsigned capacity) {
    return std::make_unique<CookieJarImpl>(capacity);
}

bool CookieJar::isEmpty() {
    return static_cast<CookieJarImpl*>(this)->isEmpty();
}

void CookieJar::fill() {
    static_cast<CookieJarImpl*>(this)->fill();
}

// CookieMonster implementation
void CookieMonster::feed(CookieJar* cookieJar) {
    while (isHungry()) {
        static_cast<CookieJarImpl*>(cookieJar)->getCookie();
    }
}

bool CookieMonster::isHungry() {
    return true;
}

This seems like a solid solution overall. It forces using a factory pattern and if you need copying and moving, you need to define the wrappers yourself in a similar fashion to the above. That is acceptable for my use case, since the classes I needed to use this for are heavyweight resources anyway.

Another interesting thing I noticed is that if you feel really adventurous, you can replace static_casts with reinterpret_casts and as long as every method of the interface is a wrapper you define, including the destructor, you can safely assign any arbitrary object to an interface you define. Useful for making opaque wrappers and other shenanigans.