2

Consider the following code parts:

this is Timing.h:

class Timing {

public:
    //creates a single instance of timing. A sinlgeton design pattern
    Timing *CreateInstance();
private:
    /** private constructor and a static instance of Timing for a singleton pattern. */
    Timing();
    static Timing *_singleInstance;
};

extern Timing timing;

and this is Timing.cpp:

Timing timing;  //a global variable of Timing

Timing *Timing::CreateInstance() {
    if (!_singleInstance) {
        _singleInstance = new Timing();
    }
    return _singleInstance;
}

Now, since I want to hold one object of Timing i made as a singleton pattern (only one timing can be created). In my exercise requirements, they say I can choose between 2 option:
create one instance of timing in the main() which is in another file and each time pass a reference of this instance to the rest of the methods in the program, or I can create a global Timing timing and state in .h file extern Timing timing. I chose the second option. However, i have some difficulties to connect between the global variable and my singleton pattern. how to i create the instance of timing in the Timing.cpp file?
i tried Timing timing = CreateInstance(), however this doesn't work..
i can't create the instance in the main() because then i will be implementing the first option..
do i need to write in main timing.CreateInstance() ?
i hope i explained my self clearly. thanks for your help

Maurice Perry
  • 32,610
  • 9
  • 70
  • 97
Asher Saban
  • 4,673
  • 13
  • 47
  • 60
  • To format code nicely, select it then press the button with 1's and 0's on it. Or indent it by four spaces. – Mike Seymour Sep 08 '10 at 15:46
  • The first option is generally better. That way, you control exactly when it's created and destroyed, and what has access to it. Singletons will give you many lifetime issues, as well as leading to a tightly-coupled design that makes testing difficult. – Mike Seymour Sep 08 '10 at 15:50
  • umm.. sorry how do i accept answers to my previous question? – Asher Saban Sep 08 '10 at 15:51
  • @rob: click the green checkmark next to the top left of the best answer. – Potatoswatter Sep 08 '10 at 17:37
  • Read this: http://stackoverflow.com/questions/1008019/c-singleton-design-pattern/1008289#1008289 – Martin York Sep 08 '10 at 18:03

2 Answers2

2

CreateInstance should be static, and you would do something like:

Timing *timing = Timing::CreateInstance();

When you need the single instance, and you would not need the timing global variable (only the static variable _singleInstance).

Oh, and as pointed out by Scott Langham, GetInstance would be a better name than CreateInstance.

Maurice Perry
  • 32,610
  • 9
  • 70
  • 97
2

You don't need to create the instance.

Change the name of CreateInstance() to GetInstance().

Then, when you want to access your timing object, call GetInstance. The singleton will be created the first time you access it.

You won't be implementing the first option if you do this because you won't keep hold of the return value from GetInstance in a variable. You'll just use it straight away.

Don't do first option and keep hold of t:

Timing* t = Timing::GetInstance();
t->DoSomething();

Do the following when you want to use your singleton.

Timing::GetInstance()->DoSomething();

(That's the basic guts of the answer anyway - technically, you can hold on to t for a while, and as long as you don't keep passing it as parameters into every function that want it, you're doing option 2)

That'll be sufficient for the task you're doing.

In production code, life times of singleton often need better control. To do that, I'd recommend having three functions:

Timing::CreateInstance()
Timing::GetInstance()
Timing::DestroyInstance()

Call Create and Destroy from the start and end of your main function. But, BE AWARE, you need to be careful that no code (even that in destructors) will access GetInstance() after you have called DestroyInstance(). I'd recommend you at least put an assert in the GetInstance function to:

assert(_singleInstance != 0)
return _singleInstance;

And, in DestroyInstance do:

delete _singleInstance;
_singleInstance = 0;
Scott Langham
  • 58,735
  • 39
  • 131
  • 204
  • Thank you. so if i understand you correctly, i need to leave Timing.cpp as is with the declaration Timing timing; and when i want to use the instance (in my main() for instance) i just need to write Timing:timing->GetInstance()->DO_SOMETHING? – Asher Saban Sep 08 '10 at 15:57
  • No, sorry, missed that. Change 'Timing timing;' to 'Timing* Timing::_singleInstance = 0;' static variables declared in the class declaration need to be defined in a .cpp – Scott Langham Sep 08 '10 at 16:03
  • Thanks you've helped a great deal! – Asher Saban Sep 08 '10 at 16:05
  • And get rid of the extern Timing timing bit – Scott Langham Sep 08 '10 at 16:06
  • it says in my exercise that the statement extern Timing timing; allows each file that #include "Timing.h" to use timing as a global variable.. so that means if I get rid of it i'll have to write Timing::timing instead? – Asher Saban Sep 08 '10 at 16:10
  • Seems you've been reading about 2 different ways to create singletons. You can either do 'extern Timing timing' in Timing.h, and access it just as 'timing.DoSomething()'. Or, you can have a static member in your timing class and access via a method like 'Timing* GetInstance()'. Doing both won't work! The GetInstance version is a bit more controllable because you can decide when the singleton gets created/destroyed as I explained. The 'extern Timing timing' way will leave the create/destroy to be handled by the C++ runtime, which is less easy to control, but will probably work fine 4 your needs – Scott Langham Sep 08 '10 at 16:15
  • that explains.. so in your opinion which way is more elegant and easy to implement the singleton pattern? – Asher Saban Sep 08 '10 at 16:19
  • The 'extern Timing timing' method is a little bit old style as it comes from the C language. The static method is more C++ and gives you more flexibility because it's easier to change in the future if you want GetInstance() to do something fancy (I'm not sure what) without changing the rest of your code, but this probably only applies if you've got a program with many 1000's of lines of code. – Scott Langham Sep 08 '10 at 16:19
  • Easiest to implement is 'extern Timing timing'. GetInstance() is more elegant as its more flexible and easier to maintain. You may find other people who disagree with me though :) – Scott Langham Sep 08 '10 at 16:22
  • error: cannot call member function ‘Timing* Timing::GetInstance()’ without object – Asher Saban Sep 08 '10 at 16:33
  • i keep getting that error above.. i need Create, Get and Destroy to be static? – Asher Saban Sep 08 '10 at 16:33
  • @rob: "which way is more elegant?" There is no elegant way to implement the Singleton pattern in C++. It should never be used. A global instance exposes you to construction-order problems; a `GetInstance()` function exposes you to thread-safety and destruction-order problems. – Mike Seymour Sep 08 '10 at 16:34
  • i understand, however i have to implement a singleton in this exercise :\ – Asher Saban Sep 08 '10 at 16:39
  • -1: Having a creation and destruction calls is horribly unlike C++ and leads to programmer error. It is the responsibility of the creator of the singleton to control lifespan not the user of the singleton (this is not Java). It is considered a bad design were the lifetime of an object has to be manually controlled and there are so many better ways to do it. – Martin York Sep 08 '10 at 18:06