Skip to content

InsulationOfImplementation

djewsbury edited this page Feb 5, 2015 · 2 revisions

#Insulation in C++

Always minimize header dependencies.

Consider insulating implementation details from clients.

##Large scale software design

XLE, like any engine, is a very large codebase. That means that physical design concepts, like those described in John Lakos' classic Large-Scale C++ Software Design, are very important.

It's worth considering how to insulation client code from implementation details for classes with many clients. This a problem that's mostly unique to C++. Other languages have built-in methods to avoid the issues shown here.

Consider the following header, let's call it "StarDatabase.h":

#include "Rendering/Shader.h"
#include "Rendering/State"
#include "Rendering/DevContext.h"
#include "Utility/BasicFile.h"

/// <summary>Imaginary class for managing stars in the sky</summary>
class StarDatabase
{
public:
  const Star* GetStar(StarId) const;
  const Constellation* Constellation(ConstellationId) const;

  StarDatabase(const ResChar sourceFileName[]);

protected:
  std::unique_ptr<Utility::BasicFile> _sourceFile;
};

/// <summary>Imaginary class with no insulation</summary>
class StarDatabase_DebuggingWidget
{
public:
  void Render(Rendering::DevContext* context);

private:
  Rendering::Shader _shader;
  Rendering::State _state;
  Rendering::DevContext::Topology _topology;

  void RenderStar(Rendering::DevContext* context, const Star&);
};

Here, StarDatabase, might be a class for reading some artist-authored star data. But within the same header there is another class, StarDatabase_DebuggingWidget. This might be a class that will render some debugging information about that star database.

But there are 2 important problems with this file:

  • there are many #includes at the top... Many headers dependencies
  • header dependencies like this lead to many redundant recompiles when headers change
  • client code has no interest in the "private" implementation details of StarDatabase_DebuggingWidget
  • but any changes or additions to that private part will still invoke recompiles of client code.

But these problems mean that small changes can invoke long recompiles. We want a system that will allow us to change things and see the results quickly and efficiently.

In other words, we want to "insulate" our clients from possible future changes. We want to hide private details away from the client, so that not only can they not access them, then can not even see them.

We can say that StarDatabase_DebuggingWidget does not insulate it's clients, because the private implementation details are part of the header.

##Redundant header includes

Where ever possible, we should avoid redundant header includes. This is one of the fundamental rules of physical architecture.

When a client adds the line: #include "StarDatabase.h" they have no reason to expect they will be inheriting dependencies on rendering headers like "Rendering/Shader.h". The client may not even use any rendering code, they might just need the StarDatabase class.

The link between "StarDatabase.h" and "Shader.h" feels illogical. It's a hint that something's very wrong.

So, how can we avoid cases like this?

###Always prefer forward declarations to #include

In the above header, #include "Utility/BasicFile" can be replaced by forward declaring Utility::BasicFile. We only refer to BasicFile via a pointer. That means we only need the name, we don't need any other information about it.

#include "Utility/BasicFile.h"

    // *becomes*

namespace Utility { class BasicFile; }

###Consider hiding implementation details with a Pimpl object

We can move the private members of StarDatabase_DebuggingWidget out of the header by using the "pimpl" idiom (or pointer-to-implementation idiom, often pronounced "pimple"). This is a strangely named, but frequently useful little trick.

// (in the header file, StarDatabase.h)
// StarDatabase_DebuggingWidget *becomes*

class StarDatabase_DebuggingWidget
{
public:
  void Render(Rendering::DevContext* context);

private:
  class Pimpl;
  std::unique_ptr<Pimpl> _pimpl;
};

// (in the source file, StarDatabase.cpp, a new class:)

class StarDatabase_DebuggingWidget::Pimpl
{
public:
  Rendering::Shader _shader;
  Rendering::State _state;
  Rendering::DevContext::Topology _topology;

  void RenderStar(Rendering::DevContext* context, const Star&);
};

In the source file, the class must access these members using the _pimpl member (eg, references to _shader become _pimpl->_shader).

Now, if we have to change any of the private members or methods of StarDatabase_DebuggingWidget, only the source file needs to change. That limits the recompile to just a single .cpp file.

Pimpl is very practical, and very useful. But there are a few things to know:

  • The default copy operator and copy constructor will no longer be valid for StarDatabase_DebuggingWidget
  • But move operator and move constructor should be fine
  • There's extra overhead in accessing members via _pimpl
  • maybe this isn't so important if _shader and _state are direct members. But if they were pointers, it adds in an extra level of indirection, which means extra overhead
  • this is a case where we trade-off some run-time performance for our physical architecture. It's not the right trade-off in all cases, but there are many cases where the run-time overhead is negligible
  • there's an extra heap block involved, since StarDatabase_DebuggingWidget exists in one heap block, but StarDatabase_DebuggingWidget::Pimpl will exist in another

We could also get the same result by hiding StarDatabase_DebuggingWidget become an abstract interface class (eg IStarDatabase_DebuggingWidget). That will generally have higher overhead costs, however there are still some cases where it's a good idea. The pimpl seems practical in a wider range of cases, however.

When using "pimpl," there are some rough guidelines:

  • generally we don't want to share _pimpl between multiple instances of the same class (ie, use std::unique_ptr<>, not std::shared_ptr<>)
  • if you want to share private data, maybe call it something other than Pimpl
  • generally Pimpl classes shouldn't be too complicated.
  • Sometimes we need to give them methods.
  • But if they get too big and complicated, there starts to be some ambiguity between what belongs on in the main class, and what belongs in the pimpl
  • To avoid this, generally keep implementation in the main class, and keep the pimpl class simple.

###Consider what classes co-exist in the same header

In this example, both StarDatabase and StarDatabase_DebuggingWidget are related, because both work with the StarDatabase concept. They are logically related. However, are they really physically related?

The 2 classes have very different dependencies.

  • StarDatabase needs to work with the file system, and maybe some serialisation utility code.
  • The StarDatabase_DebuggingWidget needs to work with the widget system, and the rendering engine.

That suggests they might exist in separate areas of the physical architecture of the engine. In this case, it might be best to separate this header into 2 separate headers, one for StarDatabase, and a separate header for StarDatabase_DebuggingWidget.

###Final header

Our final header can be re-written without any dependent header files:

namespace Utility { class BasicFile; }

/// <summary>Imaginary class for managing stars in the sky</summary>
class StarDatabase
{
public:
  const Star* GetStar(StarId) const;
  const Constellation* Constellation(ConstellationId) const;

  StarDatabase(const ResChar sourceFileName[]);

protected:
  std::unique_ptr<Utility::BasicFile> _sourceFile;
};

/// <summary>Imaginary class with insulation!</summary>
class StarDatabase_DebuggingWidget
{
public:
  void Render(Rendering::DevContext* context);

private:
  class Pimpl;
  std::unique_ptr<Pimpl> _pimpl;
};

Our new "StarDatabase.h" has a much better physical architecture. But the logical architecture of the classes hasn't changed.