I'm using the time for e.g. ping and timeout calculation as well as animations in a game. So my (big) codebase uses std::chrono::steady_clock::now() in many places. For testing I'd like to test those without having to resort to sleep and friends (timeout of >=5mins will be hard to test this way...)

For this I created a custom Clock class that uses a singleton-like instance which can be changed to provide a custom clock. A default of steady_clock is used for production code. This Clock can be used as a drop-in replacement of std::chrono::steady_clock and can be mocked in test code.

I'd like a review with suggestions how to improve this or if it has any flaws. Pointers to similar/better implementations are also welcome. I'm mostly concerned about the multiple indirections (and related performance penalties) this causes: Lookup of a "constructed" flag, the impl and the vtable (3 pointers) although I only have 1 function. See godbold

#include <chrono>

#include <memory>



struct BaseClock

{

    using Clock = std::chrono::steady_clock;

    using time_point = Clock::time_point;

    virtual ~BaseClock() = default;

    virtual time_point now(){ return std::chrono::steady_clock::now(); }

};



class Clock

{

    static std::unique_ptr<BaseClock>& inst(){

        static std::unique_ptr<BaseClock> clock(new BaseClock);

        return clock;

    }

public:

    using rep = BaseClock::Clock::rep;

    using duration = BaseClock::Clock::duration;

    using time_point = std::chrono::time_point<Clock>;



    static time_point now(){ return time_point(inst()->now().time_since_epoch()); }

    static void set(std::unique_ptr<BaseClock> clock){inst() = std::move(clock);}

};



int main()

{

    return Clock::now().time_since_epoch().count();

}

Example usage in test code:

struct MockClock: public BaseClock{

    static time_point current;

    time_point now() override { return current; }

};



void test_method(){

    Clock::set(new MockClock);

    MockClock::current = MockClock::time_point(100);

    testClass.methodUsingClock();

    MockClock::current += std::chrono::seconds(10);

    REQUIRE(testClass.checkTimeout());

}

While I think the underlying idea is quite nice, the actual implementation and design has some issues.

Wrong Abstraction Level

Let's start with the less obvious one: How would you actually use Clock or BaseClock with std::chrono::high_resolution_clock or std::chrono::system_clock?

The simplest approach would be something akin to this:

struct HighResClock : BaseClock {

    time_point now() override { return std::chrono::high_resolution_clock::now(); }

};

It seems so clean, so simple, and so easy. Except that it doesn't compile! That's because std::chrono::high_resolution_clock::time_point is not the same as BaseClock::time_point (and cannot easily be converted, if at all possible).

But: Do we actually need time_points in the public interface?

The only reason to expose time_point values is to extract time differences between them. But that only matters if arbitrary time_points are to be compared.

Technically, the time_point could be stored in some form. However, for many clocks, like std::chrono::steady_clock or std::chrono::high_resolution_clock, the epoch from when the clock are measuring their time offset can change between different executions of the same program (e.g. because the computer got rebooted).

This makes storing time_points, especially those not obtained from std::chrono::system_clock, rather useless. In that case, you'll likely need a calendar library (or similar) to get points of time in a storable format.

But in most cases, a simple Timer abstraction can fulfill all clock needs (comparing some time_points with some relation). A simple Timer interface could look like this:

struct timer {

    // choose an appropriate duration type

    using duration = std::chrono::duration<double>;



    virtual ~timer() = default;



    virtual void start() = 0;

    virtual duration stop() = 0;

    virtual duration tick() = 0; // to obtain multiple measurements from the same baseline

};



class steady_timer : public timer {

    using clock = std::chrono::steady_clock;

    using time_point = clock::time_point;



    time_point start_time;

    bool running;



public:

    steady_timer() = default;



    void start() override

    {

        start_time = clock::now();

        running = true;

    }



    duration tick() override

    {

        return duration(clock::now() - start_time);

    }



    duration stop() override

    {

        auto elapsed = tick();

        running = false;

        return elapsed;

    }

};

Now the only exposed part of the interface is the duration. And it is easily extensible to other time sources (e.g. Win32 QueryPerformanceCounter) or mockable.

Singleton

I really don't like the Clock singleton. Yes, it is easy to just ask a global clock. Yes, it is also easy to screw all code depending on this clock by changing the underlying instance.

For example, a test setting Clock to a mock but not restoring the original clock breaks all other tests that assume the default clock implementation - making test failure dependent on test execution order.

Instead, take a reference or pointer to a timer as parameter. This allows you to pass in a clock where needed, without changing (or corrupting) everyone elses timer.

Rewriting your test case:

class mock_timer : public timer {

    std::vector<duration> measurements;

    int counter = 0;



public:

    mock_timer(std::initializer_list<duration> il) : measurements(il) {}



    void start() override {}



    duration tick() override

    {

        if(counter < measurements.size()) return measurements[counter++];

        return measurements.back();

    }



    duration stop() override

    {

        return measurements.back(); // just example

    }

};



void test_method(){

    using namespace std::literals::chrono_literals;

    mock_timer my_timer{ 10s };



    testClass.methodUsingClock(my_timer);



    // or, more likely:

    testclass inst{my_timer};

    inst.methodUsingClock();



    REQUIRE(testClass.checkTimeout());

}