rahman
rahman

Reputation: 4948

cpp hpp separation

I am getting some errors when I try to put declaration and definition of a class into separatte hpp and cpp file. could you help me fix it please. I am trying to manipulate a singleton like this:

sing.hpp:

class GlobalClass {
    int m_value;
    static GlobalClass *s_instance;
    GlobalClass(int);

  public:
    int get_value();
    void set_value(int v);
    static GlobalClass *instance(); };

sing.cpp:

#include"sing.hpp"
GlobalClass::GlobalClass(int v = 0)
{
    this->m_value = v;
}

int GlobalClass::get_value()
{
    return this->m_value;
}

void GlobalClass::set_value(int v)
{
    this->m_value = v;
}

static GlobalClass GlobalClass::*instance()
{
    if (!s_instance)
        s_instance = new GlobalClass;
    return s_instance;
}

main.cpp:

#include "sing.hpp"
int main()
{
    GlobalClass *s=0;
}

command and errors are:

~/workspace/singleton$ g++  main.cpp sing.cpp 
sing.cpp: In function ‘GlobalClass GlobalClass::* instance()’:
sing.cpp:19:10: error: ‘s_instance’ was not declared in this scope
sing.cpp:2:1: error: ‘GlobalClass::GlobalClass(int)’ is private
sing.cpp:20:23: error: within this context
sing.cpp:21:12: error: ‘s_instance’ was not declared in this scope

Upvotes: 1

Views: 322

Answers (4)

Konrad Rudolph
Konrad Rudolph

Reputation: 545995

Your definition of instance has two errors:

  1. The static qualifier is erroneous.
  2. The syntax for the return type got scrambled. The pointer belongs to the type, not the name of the function:

    GlobalClass* GlobalClass::instance()
    {
        if (!s_instance)
            s_instance = new GlobalClass;
        return s_instance;
    }
    

Furthermore, you also need to define the static member s_instance as others have noted.

GlobalClass* GlobalClass::s_instance = 0;

But this code has another problem: it leaks memory. Don’t use raw pointers.

Finally, this code isn’t thread safe and this may in some situtaions be a huge problem. Assuming that you can guarantee that your code is never going to run in multi-threaded scenarios, go ahead. Otherwise, you probably want to change it (and who can offer such a strong guarantee anyway?).

Upvotes: 3

Lightness Races in Orbit
Lightness Races in Orbit

Reputation: 385325

static GlobalClass GlobalClass::*instance()
{
    if (!s_instance)
        s_instance = new GlobalClass;
    return s_instance;
}

This definition shouldn't have the static tag on it. Only the declaration.

As it is, you're not actually defining a member function; if you provided a s_instance variable you'd then get errors about that.

Also the * is in the wrong place.

You'll also later get link errors about s_instance, since you didn't define it.

Upvotes: 4

Mr Lister
Mr Lister

Reputation: 46599

Static identifiers must be defined as well as declared.

So, put s_instance in your sing.cpp. And I believe you should initialise it to NULL.

Upvotes: 1

Charles Beattie
Charles Beattie

Reputation: 5949

In sing.cpp you need to instantiate s_instance like this:

GlobalClass * GlobalClass::s_instance = NULL;

And your function static GlobalClass GlobalClass::*instance() in the cpp file shouldn't have the static keyword.

Upvotes: 1

Related Questions