Linker error on Timer based on Singleton pattern

Question

Trying to write Singleton for the first time. This is a timer that works with one function to handle timer's both start and stop and also printing result. When compiling, I'm getting linker errors like this one:

:-1: ошибка: CMakeFiles/some_algorithms.dir/timer_singleton.cpp.obj:timer_singleton.cpp:(.rdata$.refptr._ZN15timer_singleton7counterE[.refptr._ZN15timer_singleton7counterE]+0x0): undefined reference to `timer_singleton::counter'

What causes this error and how do I fix it?

Here is my source code:

timer_singleton.h

#ifndef TIMER_SINGLETON_H
#define TIMER_SINGLETON_H
#pragma once
#include <iostream>
#include <chrono>

class timer_singleton
{
public:
    timer_singleton(timer_singleton & other) = delete;
    void operator=(const timer_singleton& other) = delete;
    static timer_singleton * getInstance();
    static void hit_the_clock();
private:
    timer_singleton();
    static timer_singleton * instance;
    static std::chrono::high_resolution_clock clock;
    static std::chrono::high_resolution_clock::time_point start;
    static std::chrono::high_resolution_clock::time_point stop;
    static size_t counter;
};

#endif // TIMER_SINGLETON_H

timer_singleton.cpp

#include "timer_singleton.h"

timer_singleton::timer_singleton()
{
    clock = std::chrono::high_resolution_clock();
    start = clock.now();
    stop = clock.now();
    counter = 0;
}

timer_singleton * timer_singleton::getInstance()
{
    if (instance == nullptr)
    {
        instance = new timer_singleton();
    }
    return instance;
}

void timer_singleton::hit_the_clock()
{

    if (counter % 2 == 1)
    {
        // Clocks start ticking
        start = clock.now();
        ++counter;
    }
    else
    {
        // Clocks stop ticking and print time measured time
        stop = clock.now();
        auto duration =  std::chrono::duration_cast<std::chrono::microseconds>(stop - start);
        std::cout << "Measured time = " << duration.count() << " microseconds" << std::endl;
        ++counter;
    }

}

main.cpp

#include "timer_singleton.h"
// ...

timer_singleton * timer = timer_singleton::getInstance();
timer->hit_the_clock();
// some calculations
timer->hit_the_clock();

All of those `static` members look weird in a singleton. A singleton is already effectively `static`. Clean that up and the whole problem should go away. — user4581301, May 03 '22 at 22:26
Say, can we interest you in a nice [Meyers Singleton](https://stackoverflow.com/a/1008289/4581301)? — user4581301, May 03 '22 at 22:29
A note about `high_resolution_clock`: [What are the uses of std::chrono::high_resolution_clock?](https://stackoverflow.com/questions/37426832/what-are-the-uses-of-stdchronohigh-resolution-clock) TL;DR version: `high_resolution_clock` is weakly specified and you may find yourself with a non-monotonic clock not suited to timing. — user4581301, May 03 '22 at 22:33

user4581301 · Accepted Answer · 2022-05-03T23:25:36.107

The problem: Most non-constant static members need to be defined outside of the class definition in order to get the one-and-only-one instance that will be shared by all class instances. Normally this means that in timer_singleton.cpp you would have to add

timer_singleton::counter = 0; // allocate and initialize

But...

A singleton is effectively already static so the only static member should be the function that gets the instance. This makes the whole problem go away.

New code with comments about other useful changes:

class timer_singleton
{
public:
    timer_singleton(timer_singleton &other) = delete;
    void operator=(const timer_singleton &other) = delete;
    static timer_singleton* getInstance();
    void hit_the_clock(); // shouldn't be static
private:
    timer_singleton();

    // None of these should have been static
    std::chrono::high_resolution_clock clock; // This clock could jump around, 
                                              // including backward, in time. 
                                              // Safer with a steady_clock
    std::chrono::high_resolution_clock::time_point start;
    std::chrono::high_resolution_clock::time_point stop;
    size_t counter;
};


timer_singleton::timer_singleton():
        start(clock.now()),
        stop(start), // guaranteed to be same as start
        counter(0)
{ // assignments replaced with initializations in member initializer list
}

timer_singleton* timer_singleton::getInstance()
{ // now using Meyers singelton
    static timer_singleton instance;

    return &instance; // consider adjusting to return a reference. 
                      // Often a bit cleaner thanks to the no null guarantee
}

void timer_singleton::hit_the_clock()
{
    auto now = clock.now(); // if timing is critical, the first thing
                            // you do is get the current time.
    //if (counter % 2 == 1) // remember counter starts at 0, so first hit
                            // would stop, not start, the timer.
    if (counter % 2 == 0)
    {
        // Clocks start ticking
        start = now;
    }
    else
    {
        // Clocks stop ticking and print time measured time
        stop = now;
        auto duration = std::chrono::duration_cast<std::chrono::microseconds>(stop - start);
        std::cout << "Measured time = " << duration.count() << " microseconds" << std::endl;
    }
    ++counter; // don't repeat yourself
}

Linker error on Timer based on Singleton pattern

1 Answers1