singleton problem

[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::onlyInstance = 0;

Many Thanks!
//Tony

[Mark P]

Tony Johansson wrote:

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
static Outdoors* onlyInstance;
};
Outdoors* Outdoors::onlyInstance = 0;

Many Thanks!
//Tony

[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::onlyInstance 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::onlyInstance 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

[Prog37]

Tony Johansson wrote:

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::onlyInstance = 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::onlyInstance = NULL;

[Prog37]

Mark P wrote:

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

(You declare the constructor but never define it.)

-Mark

static Outdoors* onlyInstance;
};
Outdoors* Outdoors::onlyInstance = 0;

Many Thanks!
//Tony

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

That's why the cstor is private.

[Howard]

"Prog37" <pr****@comcast.net> wrote in message
news:0K********************@comcast.com...

Tony Johansson wrote:

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::onlyInstance = 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::onlyInstance = 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

[Mark P]

Prog37 wrote:

Mark P wrote:

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

(You declare the constructor but never define it.)

-Mark

static Outdoors* onlyInstance;
};
Outdoors* Outdoors::onlyInstance = 0;

Many Thanks!
//Tony

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.