7 replies to this topic

[bollµ]

Hi all!

I've been trying to build a Singleton template without having to change my (pretty sizable) code base. Right now, I'm passing pointers to the object to all game classes( gameObject, controllers, etc).

So, I built a singleton template. worked fine. Tried to operator overload "->". All hell broke loose >.>"

Template:

class Singleton{
private:
static  T *singleClass;
public:

static T getInstance(){
  if(singleClass == NULL)
   singleClass = new T;
  return *singleClass;
};
   T Singleton::operator ->(){
   if(singleClass == NULL)
    singleClass = new T;
   return (*singleClass);
}
};

--------------------------------------------------------------------------------------------------------------------------------------------
Class using template:

class Model : public Singleton<Model>{
private:

bool enabled;
static b2World *world;
static float gameSpeedRatio;

float stepTime;
float numStepTurns;
contactListenerManager *contactManager;
b2Body *boundaryBody;
public:
.. \\normal interface
}

--------------------------------------------------------------------------------------------------------------------------------------------
Error given byVisual Studio 2010:

1>Applicaton.obj : error LNK2001: unresolved external symbol "private: static class Model * Singleton<class Model>::singleClass" (?singleClass@?$Singleton@VModel@@@@0PAVModel@@A)
1>Model.obj : error LNK2001: unresolved external symbol "private: static class b2World * Model::world" (?world@Model@@0PAVb2World@@A)
--------------------------------------------------------------------------------------------------------------------------------------------

I know I'm doing something incredibly screwey, but I can't figure it out.
Thanks all!

[brx]

A declaration of a static class variable in C++ is just that, a declaration. You need to define it somewhere. So for the

static b2World *world;

variable you's need the line

b2World* Model::world; // initialize if desired

in your model.cpp file.

Now for the static template member I am not 100% sure. It should work to define it as

<template class T>
T* Singleton::singleClass;

inside the template header file. I never tried to give static class variables to templates.

[patrrr]

How about this:

class Singleton{
public:
static T &getInstance(){
  static  T *singleClass = NULL;
  if (singleClass == NULL)
   singleClass = new T;
  return *singleClass;
}
};

Note: I fixed a bug where you copied the instance whenever you called getInstance.
I don't understand what you want to do with your operator ->, and you don't really need world and gameSpeedRatio to be statics, do you?

PS: there's a high probability that singletons will mess with your code and design. If you find yourself passing lots of references all over the place, maybe re-thinking the design can be a good idea. There are other threads on this topic.

[rip-off]

It would be highly unusual to return the singleton by value. Such an implementation would be more of a prototype factory class rather than a singleton.

I'm not sure why you want to overload operator->() here. If you can use this operator, you already have a singleton instance. Creating a new one adds even further complications to the above issue. Think about how it will be used. If you are thinking that this will allow you to write Model->frobnicate(), it won't.

[ApochPiQ]

It may be more work in the short term, but I suggest you rewrite your code to never rely on the singleton pattern at all, or global state in any way for that matter. This will make your life much nicer in the long run.

[Ravyne]

It may be more work in the short term, but I suggest you rewrite your code to never rely on the singleton pattern at all, or global state in any way for that matter. This will make your life much nicer in the long run.

This.

In fact, your first post seems to imply that:
A) You were already passing the objects around through the dependency chain.
B) You want to use singleton because you find (A) to be inconvenient.

In truth, (A) is desirable! By choosing (B) over (A) you are actually expending a non-trivial amount of effort to create what will be a less-flexible, more-brittle, more-tightly-coupled design. In other words, quit trying to fix your singleton template, and go back to proper dependency injection.

If your dependency chain really is terribly inconvenient, then its a sign that your responsibilities are not properly thought out -- in fact, one of the reasons that Singleton is so dangerous to your code is that it allows you to cover over and ignore this code-smell -- however, Singleton does not do anything to address the underlying problem. It's like putting a fresh coat of paint on a rotting building -- looks better from the outside, but it continues to rot inside.

[bollµ]

thanks all!
And yes, the problem *was* with the static decleration.

@Ravyne: you're right, I guess I should redesign the engine to prevent parameter passing

Thanks again I've got some redesigning to do now...

[Ravyne]

Just be sure you do it right.

It's not that you want to "prevent parameter passing" as a design goal -- after all, Singleton achieves that, albeit with some rather heinous side-effects. What you want to do is redesign the engine so that responsibilities are clearly delineated, if you do that well then you will pass parameters where you need, as many times as you need -- not more, not less.

As I said, parameter passing (dependency injection) is desirable (so you can't set out to eliminate it as a design goal), but through good design you can make it so that it doesn't become a burden, and that's the sweet spot.