Software Engineering | Programming with Jim

When I first watched Miško Hevery’s Clean Code talks, I had been working for almost a year on my first large-scale programming project in the real world. I had encountered some difficulties with unit testing along the way. Some of my tests depended on nondeterministic things like interprocess communication or timers, like this camera animator class:

class CameraAngleAnimator
{
public:
    CameraAngleAnimator(double topSpeed);
    void setGoal(Quaternion const &angle);
    Quaternion currentAngle();

private:
    Clock clock_;
    Quaternion goal_;
    Quaternion lastAngle_;
    Time lastTime_;
    double topSpeed_;
};

In this class, the camera moves towards the goal up to a given top speed. Each time we’re ready to draw graphics, we query the currentAngle(). The class looks at how much time has elapsed since the last update, and moves the camera accordingly.

My unit tests looked like this:

CameraAngleAnimator animator(M_PI); // 180 degrees per second
Quaternion start(Vector(1, 0, 0), 0);
Quaternion end(Vector(1, 0, 0), M_PI);
a.setGoal(start); // first call - snaps to position immediately
a.setGoal(end);
Sleep(100); // uh-oh!
Quaternion partway = a.currentAngle();
assert(partway != start);
assert(partway != end);

This kind of unit test worked most of the time, but of course it would sometimes fail mysteriously because Sleep() is not guaranteed to be accurate, and who knows what other processes might be using up the CPU?

Furthermore, I had to keep the Sleep() durations well above the scheduler’s time slice. I think I needed about 15-millisecond intervals to get stability. A few of those tests add up to make the test suite slow.

The fix is to follow Miško’s advice, which boils down to:

Ask for your dependencies instead of constructing them yourself.

class IClock
{
    virtual Time now() = 0;
};

class CameraAngleAnimator
{
public:
    CameraAngleAnimator(double topSpeed, IClock &clock);
...
};

This allows us to make a implementation of IClock for unit testing that supplies deterministic results.

image from lisbokt

A bigger class might have 3 or 4 dependencies. For users, it’s annoying to construct those dependencies and pass them in. The code gets cluttered. Why should I deal with all this crap for the sake of their unit tests?

Java (, C#, Python, Ruby, …) programmers would typically use a factory to hide the dependencies from normal usage:

class CameraAngleAnimatorFactory
{
    public CameraAngleAnimator create(double topSpeed) {
        return new CameraAngleAnimator(topSpeed, new Clock());
    }
};

But wait a second! Before the change, the CameraAngleAnimator owned its Clock. It was a concrete, non-pointer data member. Everything was simple. No custom destructor or copy/assignment behavior was needed. Who owns the IClock now?

In garbage-collected languages, this is never an issue because every object is stored on the heap and garbage-collected when it’s not needed. C++ is not as easy.

We can’t use a concrete data member because we have no idea what class we’re storing.
A reference makes a default dependency or factory impossible: whoever owns the CameraAngleAnimator has to own the IClock as well.
An owned raw pointer begs for classic C/C++ heap errors: do you call delete in the destructor? If so, you’ll probably delete something twice by mistake, or try to delete a stack variable. If not, you’ll probably leak something. A unique_ptr is better, but still leaves the possibility of double-ownership problems.

In this simple example, we could just add a parameter double time to the currentAngle() call. Indeed, that is probably the best way to do it. But let’s imagine that the dependency is actually more complicated and not well modeled by adding method parameters.

One option is the following:

class CameraAngleAnimator
{
public:
    CameraAngleAnimator(double topSpeed) : clock_(&ownedClock_) {}
    void overrideClock(IClock &clock) { clock_ = &clock; }
    ...

private:
    Clock ownedClock_;
    IClock *clock_;
    ...
};

This version bundles a concrete Clock with each instance but lets you override it with a reference to an external IClock. If unit testing is the only reason we want dependency injection, then this is probably the way to go. However, if we need to override in the “real world”, it will not solve any ownership problems. It’s also not compatible with the Factory pattern because there’s no way for the object to take ownership of the override dependency.

Another option that’s closer to what you get in Java is a shared_ptr.

class CameraAngleAnimator
{
public:
    CameraAngleAnimator(double topSpeed, shared_ptr<IClock> clock);
    ...

private:
    shared_ptr<IClock> clock_;
    ...
};

The default dependency could come from a factory, or the .cpp implementation file could construct a default Clock if the parameter is null.

Compared to the reference/concrete version, this doesn’t waste any memory on an inactive data member when the clock is overridden. It frees the user of concern about ownership and lifetimes. But it forces a level of indirection to the heap, and it “infects” the rest of the code base with shared_ptr.

A third option is merging the two approaches:

class CameraAngleAnimator
{
public:
    CameraAngleAnimator(double topSpeed) : clock_(&ownedClock_) {}
    void overrideClock(shared_ptr<IClock> clock) {
        externalClock_ = clock;
        clock_ = clock.get();
    }
    ...

private:
    Clock ownedClock_;
    shared_ptr<IClock> externalClock_;
    IClock *clock_;
    ...
};

This version:

Carries extra memory around when overridden, but
Gets the ease-of-use of automatic memory management,
Works with the factory pattern, and
Doesn’t incur the penalty of heap allocation if the default Clock is used

It’s depressingly complex compared to the Java version though.

A completely different approach is templates, but they are not suitable for my workplace because they don’t play well with our Python and C# wrappers.

I’m undecided. One thing’s for certain: dependency injection in C++ is much less straightforward than in a garbage-collected language.

Edit, Decenber 17, 2013: Fixed a misuse of references vs. pointers.

Programming with Jim

Category Archives: Software Engineering

I Hate Singletons

Dependency Injection in C++ is hard