3

I am trying to implement singleton that I have used before in PHP and Java 8, to C++. But I do face certain restrictions from the syntax and how C++ works (specifically pointers).

This is what I have tried so far:

#include "iostream"

using namespace std;

class System{
protected:
    static System *obj;

public:
    static System *getInstance(){
        return obj;
    }

    void prn(){
        cout<<"this works!";
    }
};

int main(void){
    System &sys = System::getInstance();
    sys.prn();
}

while executing, I get the following error:

 sameer.cpp:20:10: error: non-const lvalue reference to type 'System'
 cannot bind
       to a temporary of type 'System *'
         System &sys = System::getInstance();
                 ^     ~~~~~~~~~~~~~~~~~~~~~

Please help me solve this error.. as I have no idea what it means. I have checked the forum before posting, and it can be a possible duplicate of previously asked question (which I caould not find).. But I posted this because I wanted to understand the meaning of error my code generated.

Thanks for the help

ildjarn
  • 62,044
  • 9
  • 127
  • 211
sameer
  • 81
  • 1
  • 1
  • 9
  • `System::getInstance` returns a _pointer_, but `System &sys` is a _reference_. – ForceBru Apr 20 '17 at 15:16
  • 1
    Do note that your class isn't actually a singleton. It has a default constructor and copy constructor so you can create instances and copy them. To see an actual singleton see: http://stackoverflow.com/questions/1008019/c-singleton-design-pattern – NathanOliver Apr 20 '17 at 15:16
  • 2
    try to avoid singletons as much as possible, even good explanations like this one http://stackoverflow.com/questions/137975/what-is-so-bad-about-singletons doesn't show all problems that become obvious only after long maintenance – Andriy Tylychko Apr 20 '17 at 15:16
  • 5
    You should learn C++ basics before trying to implement the Singleton pattern. – Vittorio Romeo Apr 20 '17 at 15:17
  • I know, and I am trying to learn C++, but this experiment id just out of curiosity! – sameer Apr 20 '17 at 16:07

4 Answers4

16

In C++, references and pointers are different things. A reference behaves exactly like the original variable, whereas a pointer represents the memory address of that variable. You're getting the error because you're trying to assign a pointer-to-System to a variable of type reference-to-System, which isn't the same thing. If you really wanted to you could dereference the pointer by using the System& sys = *ptr; syntax, but in this case that's the wrong thing to do; the correct fix is to return a reference from your getInstance() function, rather than a pointer.

What's more, in C++ you can actually store the static instance variable within the getInstance() function. This is a so-called "magic static", otherwise known as a "Meyers Singleton", and since C++11 it guarantees you get thread-safe construction of the singleton object. So the final solution would be:

class System
{
private:
    System() {}

public:
    static System& getInstance(){
        static System theInstance;
        return theInstance;
    }

    void prn(){
        cout<<"this works!";
    }
};

int main()
{
    System& sys = System::getInstance();
    sys.prn();
}

Also, as an aside, you should use

#include <iostream>

not

#include "iostream"

to include standard library headers. And you don't need to say main(void) in C++; empty brackets signify that a function takes no arguments, so main() will do.

Tristan Brindle
  • 16,281
  • 4
  • 39
  • 82
  • As someone already noted above, to avoid creating more than one instance (by accidentally missing & or intentionally) you also need to delete copy constructor `System(const System&) = delete;`. – Michal Jan 06 '22 at 17:43
3
#include <iostream>
using namespace std;

class System{
private:
    static System *obj;
    System() {}

public:
    static System *getInstance()
    {
        if(obj == nullptr)
            obj = new System();
        return obj;
    }

    void prn(){
        cout<<"this works!"<< endl;
    }
};

System *System::obj;

int main(void){
    System *sys = System::getInstance();
    sys->prn();
}
Ravi
  • 59
  • 3
  • 2
    This code appears almost identical to the accepted answer written by @TristanBrindle. To make this into a good and useful answer please [edit] it to explain how it solves the problem, why it is so similar to the other answer and why that difference is significant. – AdrianHHH Jul 28 '19 at 09:32
  • AdrianHHH, I will add one comment on why this difference is significant. If you are working in a C++ Builder VCL environment, the @TristanBrindle version will give an error ("[bcc32c Error]: Delphi-style classes must be allocated with 'new'") and this pattern does not give the error. C++ Builder does not let you implicitly allocate the object without the keyword "new". – Vic Fanberg May 09 '23 at 01:50
2

getInstance() returns a pointer, but you are trying to call it and bind it to a reference. If you want it to return a reference, make it return a reference.

static System &getInstance(){
    return *obj;
}
aschepler
  • 70,891
  • 9
  • 107
  • 161
-1

It might be good to lazy-initialize your singleton. Also, consider making the constructor and the singleton private so you can't create an instance anywhere else.

#include <iostream>

class System
{
private:
    static System *obj;
    System() {}
    
public:
    static System& getInstance(){
        if (nullptr == obj)
            obj = new System();

        return obj;
    }
    
    void prn(){
        std::cout << "this works!";
    }
};

int main(void){
    System& sys = System::getInstance();
    sys.prn();
}
Burak
  • 2,251
  • 1
  • 16
  • 33
AlexG
  • 1,091
  • 7
  • 15