tags:

views:

175

answers:

7

I wrote 2 classes, Agent and Timing. The third class will contain the main() function and manage it all. The goal is to create n instances of Agent and a SINGLE instance of Timing. It's important to mention that Agent uses Timing fields and Timing uses Agent functions. How can I turn Timing to singleton?

//Agent.h
#ifndef Timing_h
#define Timing_h
#include <string>
#include "Timing.h"

class Agent{
    public:
        Agent(std::string agentName);
        void SetNextAgent(Agent* nextAgent);
        Agent* GetNextAgent();
        void SendMessage();
        void RecieveMessage(double val);
            //    static Timing runTime;

What I thought would solve my problem but I got:

'Timing' does not name a type

        ~Agent();

    private:
        std::string _agentName;
        double _pID;
        double _mID;
        Agent* _nextAgent;
};
#endif


//Timing.h
#ifndef Timing_h
#define Timing_h

class Timing{
    private:
        typedef struct Message{
            Agent* _agent;
            double _id;
        }Message;
        typedef Message* MessageP;

        Message** _messageArr;
        static int _messagesCount;

    public:
        Timing();
        void AddMessage(Agent* agent, double id);
        void LeaderElected(string name);
        void RunTillWinnerElected();
        ~Timing();
};
#endif

Is this really the way to create a singleton and if it is what is the problem? And if not, how can I turn it to a singleton?

+2  A: 

Nope, the wikipedia page on the singleton pattern has a good example as well as a nice write up of how a singleton is actually constructed.

Konrad
Also note than the code exhibited does not deal with Destruction Order issues or multithreading inconsistencies.
Matthieu M.
A: 

Define a static member of Agent that is of type Timing and initialize the member where it is defined. This is not strictly a Singleton any more but may model the behaviour you want better.

Steve Townsend
+1  A: 

A singleton has to have a private constructor, and then a static method to get an instance of the class, which is a private field in the class itself.

vulkanino
+9  A: 

This bit looks suspicious in Agent.h ...

#ifndef Timing_h
#define Timing_h

Seems like it's the same define-guard in Timing.h. Therefore Timing will not be included in Agent.

Change it to

#ifndef Agent_h
#define Agent_h
Vite Falcon
+1  A: 

Since somebody already mentioned the wikipedia page, I'll also mention another implementation of the Singleton pattern done slightly differently.

If you look at the Ogre::Singleton class you'll see it done in a different way. It allows you to only call the constructor once (or an assertion happens). So in your setup, you call the constructor. Then at the end of the program, you get this instance and delete it.

It allows you to make a singleton instance, but allow it to be instantiated with different parameters. I don't like it nearly as much as the wikipedia implementation though since it requires you to manage the constructor/destructor of the singleton.

Jonathan Sternberg
2 notes: the code forgo lazy initialization (which should solve multithreading issues) but it means you need to explicitly instantiate both `ms_singleton` variable (to 0) and your singleton. Furthermore you may have crash depending on the initialization order / destruction order which varies from build to build (though it should be diagnosed thanks to the null pointer).
Matthieu M.
+2  A: 

Your Timing is not a singleton. Multiple objects can be created. Singleton's typically rely on a static method and a private ctor and no copy ctor or op=. You have two choices:

  • Either create a static member Timing for Agent
  • Or, create a Timing& member for Agent and convert Timing to a proper singleton

Depending on your design, if the Timing object is never changed by the Agent objects you may go ahead const qualify the member.

dirkgently
I chose the second way, with wiki help, but how can i use Timing from Agent? i tried to create a Timing field in Agent.h but i couldn't.how can i use the getInstance() method from Timing?
or.nomore
dirkgently
but when i declare a member: Timing timing, in the Agent.h filei get error: 'Timing' does not name a type
or.nomore
You need a forward declaration for `Timing` or will have to `#include` the header file that defines `Timing`.
dirkgently
ok, i forgot that the getInstance() is statis so i have to use:(Timing::getInstance()).somthing..
or.nomore
+1  A: 

The most conventional way to create a singleton has following requirements:

1) The constructor should be private and a static interface shall be provided which in turn creates a static object of the singleton class (which is a member of the class itself) and return it. Basically provide a global point of access.

2) You have to decide in advance if you want the users of the class to be able to extend it and design your class to support it if required.

If the above two requirements are met, there are a number of ways a singleton can be designed including the ones which would work with multi threaded applications.

hype
For a list of gotch's about singelton: http://stackoverflow.com/questions/1008019/c-singleton-design-pattern/1008289#1008289
Martin York