Singelton Pattern Unsave?

M

Marcin Vorbrodt

Hello dear fellows C++ coders!
I saw a discussion here before about static memebr functions and doing
singelton pattern. It was sugested that method like this was unsave (example
from my code):

class Vector {
Vector(float, float, float);
...
static const Vector UNIT_X(void);
};

const Vector Vector::UNIT_X(void) {
static const Vector unitX(1.0, 0.0, 0.0);
return unitX;
}

Instead, it was recomende by someone to do this:

const Vector Vector::UNIT_X(void) {
static const Vector *unitX = new Vector(1.0, 0.0, 0.0);
return *unitX;
}

Could some please explain exactly why that might be? Also, if the static
object is const, could I return by reference to the static object, like
this:

const Vector& Vector::UNIT_X(void) {
static const Vector *unitX = new Vector(1.0, 0.0, 0.0);
return *unitX;
}

This would be faster (no need for a temporary), but would it be safe to
return a reference? Or should I stick with returning a new copy each time?

Thanks a bunch!

Marcin
 
V

Victor Bazarov

Marcin Vorbrodt said:
I saw a discussion here before about static memebr functions and doing
singelton pattern. It was sugested that method like this was unsave (example
from my code):

Who suggested that? If it were suggested here, why not continue in
the same thread?
class Vector {
Vector(float, float, float);
...
static const Vector UNIT_X(void);

'void' instead of the argument list is C. In C++ you don't need it.
Just write '()'. Much easier to read.
};

const Vector Vector::UNIT_X(void) {
static const Vector unitX(1.0, 0.0, 0.0);
return unitX;

This is nonsense. Why do you return by value? Much more efficient
would be to return by reference. Besides, if you return by value,
there is no singleton per se. You always create another instance of
Vector (by copying the static one).
}

Instead, it was recomende by someone to do this:

const Vector Vector::UNIT_X(void) {
static const Vector *unitX = new Vector(1.0, 0.0, 0.0);
return *unitX;

By whom? Again, try not to start unnecessary threads, keep the
discussion going. Ask questions right there and then.
}

Could some please explain exactly why that might be?

Why what might be? Unsafe? Bullshit. There is no safety involved
here. Take a look:

Vector unit_X = Vector::UNIT_X();

No matter what you return from UNIT_X() member function, I can change
the 'unit_X' object and you cannot do anything about it. I can shoot
myself in the foot if I want to, there is nothing you can do to stop me.

If there is any other safety that you can think of, please elaborate.
Also, if the static
object is const, could I return by reference to the static object, like
this:

You not only _could_, you _should_.
const Vector& Vector::UNIT_X(void) {
static const Vector *unitX = new Vector(1.0, 0.0, 0.0);
return *unitX;
}

This would be faster (no need for a temporary), but would it be safe to
return a reference? Or should I stick with returning a new copy each time?

Where's that concern of safety coming from? What could be _unsafe_
about returning a reference to a const object? BTW, creating a pointer
and dereferencing it is (a) dangerous because you don't check for any
errors (what if there isn't enough memory to allocate a Vector?) and
(b) sloppy because you never free the memory you allocate.

Victor
 
M

Marcin Vorbrodt

Victor Bazarov said:
Who suggested that? If it were suggested here, why not continue in
the same thread?


I dont think that thread is still on my news server.

'void' instead of the argument list is C. In C++ you don't need it.
Just write '()'. Much easier to read.


Will keep that in mind.

This is nonsense. Why do you return by value? Much more efficient
would be to return by reference. Besides, if you return by value,
there is no singleton per se. You always create another instance of
Vector (by copying the static one).


By whom? Again, try not to start unnecessary threads, keep the
discussion going. Ask questions right there and then.


Why what might be? Unsafe? Bullshit. There is no safety involved
here. Take a look:


Yes, but i am returning a reference to memeory (read only memory, but
still).
Is there a possibility some would cast away the constness and overwrite the
value? Or would that result in a runtime error/core dump?

Vector unit_X = Vector::UNIT_X();

No matter what you return from UNIT_X() member function, I can change
the 'unit_X' object and you cannot do anything about it. I can shoot
myself in the foot if I want to, there is nothing you can do to stop me.

If there is any other safety that you can think of, please elaborate.


You not only _could_, you _should_.
time?

Where's that concern of safety coming from? What could be _unsafe_
about returning a reference to a const object? BTW, creating a pointer
and dereferencing it is (a) dangerous because you don't check for any
errors (what if there isn't enough memory to allocate a Vector?) and
(b) sloppy because you never free the memory you allocate.


Safety... one I just mentioned... returning effectively a memory address,
although i dont know if it would/could be modified.

Two... those are static objects. Static objects on a stack get destroyed
before the program exits right? The order of destruction (just like the
order of creation of static objects between compilation units) is unknown.
What IF (really HUGE IF), at destruction of some static object, that object
depends on some other static object... which has allready been destroyed.
Then shit hits the fan, doesn't it? Say a destructor for some other class
does something along those lines...

~OtherClass() {
.... ble ble ble...
.... Vector::UNIT_X();
}

and this code happend during destruction of OtherClass static variable.
Well, the static inside UNIT_X could have been destroyed allready, right?

Putting it on the heap assures it will exist past the point of program
termination... OS will release the memory.
 
M

Marcelo Pinto

Victor Bazarov said:
BTW, creating a pointer
and dereferencing it is (a) dangerous because you don't check for any
errors (what if there isn't enough memory to allocate a Vector?) and
(b) sloppy because you never free the memory you allocate.

Victor

I believe he doesn't have to check for errors because new throws an
exception if it cannot allocate memory.

Regards,

Marcelo Pinto
 
M

Marcelo Pinto

Marcin Vorbrodt said:
Hello dear fellows C++ coders!
I saw a discussion here before about static memebr functions and doing
singelton pattern. It was sugested that method like this was unsave (example
from my code):

As Victor pointed out, you should have posted a follow up to that
thread
class Vector {
Vector(float, float, float);
...
static const Vector UNIT_X(void);
};

const Vector Vector::UNIT_X(void) {
static const Vector unitX(1.0, 0.0, 0.0);
return unitX;
}

As I see it, your problem is not quite about singleton. IMO, what you
are trying is to keep a "constant" avaiable to all the users of your
class (1,0,0). Probably you will want (0, 1, 0) and (0, 0, 1).
Instead, it was recomende by someone to do this:

const Vector Vector::UNIT_X(void) {
static const Vector *unitX = new Vector(1.0, 0.0, 0.0);
return *unitX;
}

I believe that this recommendation came from the book of GoF which
uses this dialect to create singletons.
Could some please explain exactly why that might be? Also, if the static
object is const, could I return by reference to the static object, like
this:

const Vector& Vector::UNIT_X(void) {
static const Vector *unitX = new Vector(1.0, 0.0, 0.0);
return *unitX;
}

You could mix the two solutions (use return by reference of a static
local object)

const Vector & Vector::UNIT_X(void) {
static const Vector unitX(1.0, 0.0, 0.0);
return unitX;
}
This would be faster (no need for a temporary), but would it be safe to
return a reference? Or should I stick with returning a new copy each time?

Thanks a bunch!

Marcin

My incomplete solution to your problem would be:

//Vector.h
class Vector
{
public:
Vector(double x, double y, double z):_x(x), _y(y), _z(z) {}
double & X() { return _x; }
double & Y() { return _y; }
double & Z() { return _z; }
const double & X() const { return _x; }
const double & Y() const { return _y; }
const double & Z() const { return _z; }

static const Vector & Unit_X();
private:
double _x, _y, _z;
};

//Vector.cpp
const Vector & Vector::Unit_X()
{
static const Vector UX(1.0, 0.0, 0.0);
return UX;
}

Best Regards,

Marcelo Pinto
 
V

Victor Bazarov

Marcin Vorbrodt said:
[...]
Safety... one I just mentioned... returning effectively a memory address,
although i dont know if it would/could be modified.

As I said before, if I want to shoot myself in the foot, you can't
stop me. Returning a reference to a const vector is efficient, it
has some protection built in, and it's the best you can do.
Two... those are static objects. Static objects on a stack get destroyed
before the program exits right? The order of destruction (just like the
order of creation of static objects between compilation units) is unknown.

The order of destruction is the reverse of the order of creation.
What IF (really HUGE IF), at destruction of some static object, that object
depends on some other static object... which has allready been destroyed.
Then shit hits the fan, doesn't it? Say a destructor for some other class
does something along those lines...

~OtherClass() {
... ble ble ble...
... Vector::UNIT_X();
}

and this code happend during destruction of OtherClass static variable.
Well, the static inside UNIT_X could have been destroyed allready, right?

Unknown. However, you are right to be concerned. And just like with
many other things, if this can happen, you may want to protect yourself
or your user and step away from using a static variable. If returning
a value is not expensive (and you can only find out by profiling the
code after the entire program is written), return a value.
Putting it on the heap assures it will exist past the point of program
termination... OS will release the memory.

Relying on OS to release the memory is non-portable.

Victor
 
J

Jack Klein

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
474,141
Messages
2,570,818
Members
47,367
Latest member
mahdiharooniir

Latest Threads

Top