singleton problem

T

Tony Johansson

Hello!

I have a class called Outdoors which is a singleton see below for definition
and a main that call this instance method.

Now to my problem this piece of code doesn't compile I get the following
error I can't understand what it is.
start.obj : error LNK2001: unresolved external symbol "private: __thiscall
Outdoors::Outdoors(void)" (??0Outdoors@@AAE@XZ)
Debug/slask.exe : fatal error LNK1120: 1 unresolved externals
Error executing link.exe.


#include "outdoors.h"
int main()
{
Outdoors* base = Outdoors::instance();
return 0;
}

//outdoors.h
//********
class Outdoors
{
public:
static Outdoors* instance()
{
if (onlyInstance == 0)
onlyInstance = new Outdoors();

return onlyInstance;
}

private:
Outdoors();
static Outdoors* onlyInstance;
};
Outdoors* Outdoors::eek:nlyInstance = 0;

Many Thanks!
//Tony
 
M

Mark P

Tony said:
Hello!

I have a class called Outdoors which is a singleton see below for definition
and a main that call this instance method.

Now to my problem this piece of code doesn't compile I get the following
error I can't understand what it is.
start.obj : error LNK2001: unresolved external symbol "private: __thiscall
Outdoors::Outdoors(void)" (??0Outdoors@@AAE@XZ)
Debug/slask.exe : fatal error LNK1120: 1 unresolved externals
Error executing link.exe.


#include "outdoors.h"
int main()
{
Outdoors* base = Outdoors::instance();
return 0;
}

//outdoors.h
//********
class Outdoors
{
public:
static Outdoors* instance()
{
if (onlyInstance == 0)
onlyInstance = new Outdoors();

return onlyInstance;
}

private:
Outdoors();

Try replacing above line with: Outdoors() {};

(You declare the constructor but never define it.)

-Mark
 
R

Rade

Now to my problem this piece of code doesn't compile I get the following
error I can't understand what it is.
start.obj : error LNK2001: unresolved external symbol "private: __thiscall
Outdoors::Outdoors(void)" (??0Outdoors@@AAE@XZ)
Debug/slask.exe : fatal error LNK1120: 1 unresolved externals
Error executing link.exe.

You have declared the default constructor for Outdoors (as private), and you
are using it (when creating the instance), but you have not defined it.
Define it (at least as empty, unless you have any logic to put in it), and
the error will disappear.

BTW there are other errors:

1) You have *defined* Outdoors::eek:nlyInstance in the header. This would mean
that if the header is included in two modules, there will be multiple
definition of the same object, which is forbidden (and will most probably
cause the linker to complain). Please create a new module (e.g.
outdoors.cpp) and define Outdoors::eek:nlyInstance there.

2) Your code never destroys the singleton. That may be what is desired, but
be aware of that fact. You will have at least a memory leak for the
singleton object, if not other leaks. As an example, if the singleton keeps
a file open and writes to it, and if it closes the file in its destructor,
the file may be closed by the OS instead of your application, which means
that some data may be buffered and not written at the end, i.e. the file may
get corrupted. One can easily think of more examples whenever the Outdoors
destructor is nontrivial.

3) It is too easy to delete the singleton (e.g. delete
Outdoors::instance()).

To solve problems 2 and 3, usually the following pattern is used:

// outdoors.h
class Outdoors
{
public:

static Outdoors &Outdoors::instance();
// etc.
private:
Outdoors() { /* ... */ } // note that this is a definition as well
// etc.
};

// outdoors.cpp
Outdoors &Outdoors::instance()
{
static Outdoors onlyInstance;
return onlyInstance;
}

Note that instance() returns a reference instead of pointer, so it would be
harder to delete it inadvertently. C++ Standard guarantees that Outdoors
constructor will be called first time the instance() method is called, and
destructor will be called upon returning from main(). However, there is also
a couple of pitfalls with this approach: (1) it is unspecified when exactly
after main() the singleton will be destroyed; particularly this is important
if you have multiple singletons depending on each other, (2) possible
threading issues which are OT here.

Rade
 
P

Prog37

Tony said:
Hello!

I have a class called Outdoors which is a singleton see below for definition
and a main that call this instance method.

Now to my problem this piece of code doesn't compile I get the following
error I can't understand what it is.
start.obj : error LNK2001: unresolved external symbol "private: __thiscall
Outdoors::Outdoors(void)" (??0Outdoors@@AAE@XZ)
Debug/slask.exe : fatal error LNK1120: 1 unresolved externals
Error executing link.exe.


#include "outdoors.h"
int main()
{
Outdoors* base = Outdoors::instance();
return 0;
}

//outdoors.h
//********
class Outdoors
{
public:
static Outdoors* instance()
{
if (onlyInstance == 0)
onlyInstance = new Outdoors();

return onlyInstance;
}

private:
Outdoors();
static Outdoors* onlyInstance;
};
Outdoors* Outdoors::eek:nlyInstance = 0;

Many Thanks!
//Tony
When you declare a static member variable in a class you need to
define the variable somewhere to avoid a linker error.

Put the following at the top of your .cpp file and the link error
will go away.

Outdoors* Outdoors::eek:nlyInstance = NULL;
 
P

Prog37

Mark said:
Try replacing above line with: Outdoors() {};

(You declare the constructor but never define it.)

-Mark

It's a SINGLETON he does not want anyone to be able to new an instance
directly.

That's why the cstor is private.
 
H

Howard

Prog37 said:
When you declare a static member variable in a class you need to
define the variable somewhere to avoid a linker error.

Put the following at the top of your .cpp file and the link error
will go away.

Outdoors* Outdoors::eek:nlyInstance = NULL;

He's already got that, but in the wrong place. It shouldn't be in a header
file, but in an implementation file, such as Outdoors.cpp.

But the link error he's getting is related to the constructor, which is not
defined. One solution is to add an empty implementation of the constructor
in the code above. But Rade suggests a better alternative, I think.

-Howard
 
M

Mark P

Prog37 said:
It's a SINGLETON he does not want anyone to be able to new an instance
directly.

That's why the cstor is private.

What has that got to do with my advice? If you use new, you better have
a constructor, and if you also declare a constructor, you better define it.
 

Ask a Question

Want to reply to this thread or ask your own question?

You'll need to choose a username for the site, which only take a couple of moments. After that, you can post your question and our members will help you out.

Ask a Question

Members online

Forum statistics

Threads
473,780
Messages
2,569,611
Members
45,270
Latest member
TopCryptoTwitterChannels_

Latest Threads

Top