0

Dear StackOverflowers,

I made a singleton class called Log, but it does not call the constructor. The class is used to print Serial commands depending on m_LogLevel. I want the constructor to be called once when getInstance() is called for the first time. For some reason though, the constructor never seems to be called, what am I doing wrong?

Thanks in advance!

Log.h:

class Log {

  private:
    int m_LogLevel = LOGLEVEL;
    static Log* instance;
    Log();
    ~Log();

  public:
    static Log* getInstance();
    void debug(String);
};

Log.cpp:

Log* Log::instance = nullptr;

Log::Log() {
  DEBUG_PRINT("Log level: ");
  DEBUG_PRINTLN(m_LogLevel);
}

Log::~Log() {
}

Log* Log::getInstance() {
    if (instance == 0) {
        instance = new Log();
    }
    return instance;
}

void Log::debug(String message) {
  if(m_LogLevel >= LogLevelDebug) {
  DEBUG_PRINT("[DEBUG]: ");
  DEBUG_PRINTLN(message);
}

}

main.cpp:

#define DEBUG_PRINT(x)  Serial.print (x)
#define DEBUG_PRINTLN(x)  Serial.println (x)
#define LOGLEVEL 3

#include <Arduino.h>
#include "Log.h"

Log* pLog = Log::getInstance();

void setup() {
  Serial.begin(115200);
  pLog->debug("Hello world");
}

void loop() {
}
Daan van Driel
  • 165
  • 1
  • 2
  • 9
  • Please provide a [MCVE], the code you show seems to be OK (more or less). – πάντα ῥεῖ Nov 10 '18 at 13:04
  • 1
    Worth looking at (related) https://stackoverflow.com/a/1008289/3807729 – Galik Nov 10 '18 at 13:11
  • 2
    To create your [Compilable, Minimal, Complete, and Verifiable Example](https://stackoverflow.com/help/mcve), please be sure to include a main() function and all include statements so that others can cut and paste your code and compile it without having to do any additional typing. This helps us help you. If you can make your problem as simple as possible while still creating the failure, it will help us isolate the issues that will make your code work correctly. – Gardener Nov 10 '18 at 13:28
  • 2
    Can you confirm that your code is not multithreaded (because instance is not atomic so that there could be a data race) ? Also, can you make the same test by using cout instead of your DEBUG_XXX, just to be sure that it's just a problem in the output ? FInally, could you show how you use your getInstance() ? – Christophe Nov 10 '18 at 13:40
  • 1
    `Log* Log::instance = 0;` is wrong! You should assign pointers with `nullptr` instead of just `0`.... – Ruks Nov 10 '18 at 13:40
  • 1
    I suspect that the `pLog` global is initialized after the code tries to access the instance. By using a global variable you are subverting the only thing that makes singletons better than a global variable - namely guaranteed construction before the first use. – Michael Veksler Nov 10 '18 at 14:02

0 Answers0