On Sep 7, 10:24 am, "Chris M. Thomasson" <
[email protected]>
wrote:
During a code review, I found the following lines of
code:
[...]
The "instance" method was implemented as follows:
Data* Data::instance()
{
static Data* model = new Data();
return model;
}
I have never come across a situation where a pointer was
set to static in such a case. Is this valid?
It's a singleton.
What are the potential pitfalls in such programming
practices?
The storage that `model' points to will never be
destroyed, also it's not thread-safe.
I was wondering about the static pointer part - I have
always seen static variables (that are not pointers) in use,
but never a static pointer (even if it is to guarantee that
the singleton always returns the *same* instance of the
Class). Is a static pointer (as in the instance function) a
perfectly valid use of the "static" keyword?
A pointer is an object, just like any other type, and a variable
of pointer type follows exactly the same rules as a variable of
any other type.
I think it is. After all, that pointer should be initialized
only once and operator new should be called only once - the
first time the method is invoked - but this confirmation
should be implied into Chris' sentence: "It's a singleton".
Maybe. The fundamental definition of a singleton is that the
class can only have one instance. The example just happens to
be one of the more frequent implementation techniques.
As usual, there are many ways to achieve the same target while
programming.
And many ways to achieve something that is almost the same
thing, but subtly different.
But was that coder really meant to implement it that way?
The implementation you reviewed:
-------
Data* Data::instance()
{
static Data* model = new Data();
return model;
}
-------
Gives the same result[1] of:
-------
Data* Data::instance() {
static Data model;
return &model;
}
-------
Not a all. In your version, the singleton will be destructed.
Possibly too early. In his version, the singleton will never be
destructed. In most cases, his version is to be preferred.
[1] Well, more or less "the same result". My mod could be
preferred because it doesn't use dynamic memory, but in order
to avoid some client to see the pointer as something that
could/should be deleted sooner or later, the code could return
the object as a reference.
That's an orthogonal issue, and typically, the instance function
of a singleton does return a reference, regardless of the
implementation behind it. That, of course, doesn't prevent the
client from deleting it---often, the destructor will also be
made private as well. But typically, a reference will be used
because the client code isn't expected to delete it, and the
function can never return a null pointer.
Changing the method declaration, it could be implemented it in this
way:
-------
Data& Data::instance() {
static Data model;
return model;
}
-------
or also as:
-------
// data.cpp
#include "data.h"
namespace {
Data model;
}
Data& Data::instance() {
return &model;
}
And doesn't avoid the order of initialization issues the other
versions were designed to avoid.