private-implementation pattern

[jfirebaugh]

With the styles API, a lot more API surface area will be publicly exposed:

• Style
• Source and future subclasses for specific source types
• StyleLayer and subclasses for specific layer types
• Paint and layout properties for all layer types

So far, we've been using the following technique for separating public APIs from private implementation:

• Public headers live in include, private headers live in src
• Public headers use forward declarations and pointers to avoid needing to include private headers

For the styles API, I don't think this is going to be enough. For example, I don't think we want to simply make style_layer.hpp a public include file -- it would cascade into making other headers public as well, and expose implementation details we want to be free to change. The current StyleLayer and subclasses depend on rapidjson headers (not easily forward declared) and have methods that are conceptually public to other mbgl internal classes, but private to API consumers (e.g. cascade, recalculate, createBucket).

Therefore we need to use more heavy-duty techniques to separate the public API from private implementation. I propose we adopt a particular implementation of the opaque pointer pattern, and follow this convention throughout portions of the codebase exposed to the public API:

• Public header files expose objects in the mbgl namespace. (Already doing this.)
• These objects hold a pointer to a corresponding instance of a class with the same name, but in the mbgl::impl namespace.
• All implementation code moves to the mbgl::impl namespace and src/mbgl/impl directory tree.

Example:

include/mbgl/map.hpp

namespace mbgl {
namespace impl { class Map; }

class Map {
public:
    void doSomething();

private:
    std::unique_ptr<impl::Map> impl;
};

}

src/mbgl/map.cpp

namespace mbgl {

void Map::doSomething() {
   impl->a.doSomething();
   impl->b.doSomethingElse();
}

}

src/mbgl/impl/map.hpp

#include <mbgl/map.hpp>
#include <mbgl/impl/foo.hpp>
#include <mbgl/impl/bar.hpp>

namespace mbgl {
namespace impl {

class Map {
public:
    Foo a;
    Bar b;
};

}
}

In this example I've shown implementing mbgl::Map::doSomething in src/mbgl/map.cpp. Another option is to forward to mbgl::impl::Map::doSomething and put the real implementation in src/mbgl/impl/map.cpp. But that's another level of indirection and I don't know if there's any benefit.

Thoughts?

[jfirebaugh]

Here's a second option, without an impl namespace/subdirectory. I think I like this better. It's similar to our pattern for Thread.

include/mbgl/map.hpp

namespace mbgl {

class Map {
public:
    void doSomething();

private:
    class Impl;
    std::unique_ptr<Impl> impl;
};

}

src/mbgl/map.cpp

namespace mbgl {

void Map::doSomething() {
   impl->a.doSomething();
   impl->b.doSomethingElse();
}

}

src/mbgl/map_impl.hpp

#include <mbgl/map.hpp>
#include <mbgl/foo.hpp>
#include <mbgl/bar.hpp>

namespace mbgl {

class Map::Impl {
public:
    Foo a;
    Bar b;
};

}

[jfirebaugh]

Bleah... this is going to be a drag no matter how it's implemented: c510c68.

[daniel-j-h]

@jfirebaugh approaching the pimpl idiom by means of std::unique_ptr and std::make_unique in the ctor initializer list is great! Just wanted to leave this Meyers advice here:

Item 22: When using the Pimpl Idiom, define special member functions in the implementation file
Meyers, Effective Modern C++

The item is too many pages to repeat here in detail; please look it up in your local Effective Modern C++ copy or get one asap :) The tl;dr is this: you have to declare the special member functions like copy ctor and copy assign operator in the interface, and then define them appropriately in the implementation file. Default behavior is not what you want.

Sidenote: for implementations that have to live in header files, the idiomatic way to to this is to have a detail namespace (potentially in a local detail subdirectory), and include those from the main header.

[tmpsantos]

@jfirebaugh I like the class Impl used for Thread. This is already how we do for AsyncTask, Timer, etc.