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();

CodePudding user response：

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
}