Help with a singleton class

[Absurd]

Hi.
I'm trying to create a singleton class that will save radio statistics.
here's how it looks:

#include<iostream>
#include<string>
#include<map>
#include<iterator>
#include<algorithm>
#include"Song.h"
#include"Playlist.h"
using namespace std;



class RadioStatistics
{
private:
    static RadioStatistics* m_instance;
    static map<string, int> m_songs;
    static map<string, int> m_singers;
    RadioStatistics();
public:
    static RadioStatistics* getInstance();
    string getMostPlayedSong();
    string getMostPlayedSinger();
};

the idea is to increment the value of m_songs and m_singers whenever a song is played in the radio, so later it'll be possible to know what song was played the most and who's the most popular singer.

here's some of the implementation so far:

#include "RadioStatistics.h"


RadioStatistics::RadioStatistics()
{
    cout<<"CTR called"; //indicator
}


RadioStatistics* RadioStatistics::getInstance()
{
    if (!m_instance) //an error here
        m_instance=new RadioStatistics; 
    return m_instance;
}


bool CompareValues(pair<string, int> lhs, pair<string, int> rhs)
{
    return lhs.second<rhs.second;
}


string RadioStatistics::getSongMostPlayed()
{
    pair<string, int> tempPair=*max_element(m_songs.begin(),m_songs.end(),CompareValues); //an error here
    return tempPair.first;
}

the problem is:

undefined reference to `RadioStatistics::m_instance'
undefined reference to `RadioStatistics::m_instance'
undefined reference to `RadioStatistics::m_instance'
undefined reference to `RadioStatistics::m_songs'
undefined reference to `RadioStatistics::m_songs'

can anyone please shed some light on that issue?
why is it undefined?

thank you in advanced.

[ZuK]

You have only declared the static members minstance, m_songs_m_singers. You have to define them as well in the .cpp
e.g.

map<string, int>  RadioStatistics::m_songs;
...

Kurt

[phantomotap]

O_o

You don't really have a "Singleton". You just have a collection of global variables in the `RadioStatistics' namespace.

That said, why do you need a "Singleton"?

Search for using `static' variables in a class.

Soma

[Absurd]

You have only declared the static members minstance, m_songs_m_singers. You have to define them as well in the .cpp
e.g.

map<string, int>  RadioStatistics::m_songs;
...

Kurt

why is it different from when I define a data members within any other class?

O_o

You don't really have a "Singleton". You just have a collection of global variables in the `RadioStatistics' namespace.

That said, why do you need a "Singleton"?

Search for using `static' variables in a class.

Soma

I need a singleton because these are the assignment requirements...

[phantomotap]

why is it different from when I define a data members within any other class?

O_o

Why would you expect a `static' class variable, a variable disconnected from any instance, to be the same as a class member variable?

I need a singleton because these are the assignment requirements...

The assignment makes me sad. ;_;

I guess you have to do the assignment, but keep this in mind for the future: generally speaking, you should just create a single instance of a class if you only need one.

Soma

[ZuK]

why is it different from when I define a data members within any other class?

Because there is supposed to be only a single instance.
Kurt

[Absurd]

The assignment makes me sad. ;_;

you and me both.

I guess you have to do the assignment, but keep this in mind for the future: generally speaking, you should just create a single instance of a class if you only need one.

Soma

and this is exactly what I need to achieve here.
It actually makes sense to have a single instance of an object that suppose to gather statistics for the whole program.

Because there is supposed to be only a single instance.
Kurt

How do I define it then?
What is the sintax?

edit...

I found this to be useful:

RadioStatistics* RadioStatistics::m_instance=NULL;

solved the error for m_instance, but what about m_songs and m_singers?

[Elysia]

Same thing. Initialize them to nullptr if your compiler supports C++11 (because NULL is evil).
Make sure you properly delete those instances at the end of your program, too (or use smart pointers if your compiler supports C+11/TR1).

[ZuK]

but what about m_songs and m_singers?

see #2
Kurt

[Absurd]

Perfect!

thank you so much.

Elysia, our assignments checked with an automatic input/output system, and I don't know much about its compiler specifications,
so I prefer not using any extensions at this moment.

[Elysia]

Elysia, our assignments checked with an automatic input/output system, and I don't know much about its compiler specifications,
so I prefer not using any extensions at this moment.

They are not extensions. They are part of the language.
Unfortunately, not all compilers up to snuff, so features added more recently (or more specifically, recent standards) have not gotten implemented in all compilers.
Of course, you are not required to use them. I am simply making you aware of that they exist.

[phantomotap]

It actually makes sense to have a single instance of an object that suppose to gather statistics for the whole program.

O_o

Sure. It just doesn't make any sense for the object to be purpose build as a "Singleton".

Think of expanding the project: say you want to have a "most played song of the day" or "most played song of the week".

Don't you think it would be silly to make a `RadioStatistics' for each frame of time?

That was my point. You don't need a "Singleton" to only have a single instance or a globally accessible instance. The compiler isn't going to go around creating objects you don't ask to be created. If you only need a single instance, just create a single instance of an object and use it.

Soma

[Absurd]

O_o

Sure. It just doesn't make any sense for the object to be purpose build as a "Singleton".

Think of expanding the project: say you want to have a "most played song of the day" or "most played song of the week".

Don't you think it would be silly to make a `RadioStatistics' for each frame of time?

That was my point. You don't need a "Singleton" to only have a single instance or a globally accessible instance. The compiler isn't going to go around creating objects you don't ask to be created. If you only need a single instance, just create a single instance of an object and use it.

Soma

what would be an appropriate use of a Singleton then?

[whiteflags]

People debate the usefulness of the singleton pattern as a whole, mainly because the singleton is overused, and not entirely necessary. The singleton is just a wrapper around a global instance 99% of the time. The whole point of criticism is that it isn't useful.

[phantomotap]

what would be an appropriate use of a Singleton then?

O_o

I'm not sure that any real cases for "Singleton" classes exist, but if any do exist, they would certainly be very specific cases, and even then, the need for a class to be built as a "Singleton" is imaginary as what you've done in building a "Singleton" is easily isolated allowing you to get a normal class and a "Singleton" facility at the same time.

static SNormalObject * g = 0;
SNormalObject * GetNormalObject()
{
    MLazy<SNormalObject>::type s(g);
    return(g);
}

I will not give you code, but the point is that `MLazy' is a facility that lazily constructs a `SNormalObject' on behalf of `GetNormalObject'.

If you need a normal instance, you can use `SNormalObject' like a normal class.

If you need the globally accessible object, without "C++ Order Fiasco" problems, use `GetNormalObject' as you would for a `Singleton'.

Even this sort of thing is rarely necessary, but it is valuable in that it solves real problems without purpose building "Singleton" classes.

class SObject
{
    // ...
    SObject
    (
        /**/
      , SNormalObject * fObject = GetNormalObject()
    );
    // ...
};

SObject g1; // This just works; the `GetNormalObject' is known to "live" an appropriate "life".

The point is, you haven't built a "Singleton"; you've created a single instance of an object which you are just using from a global point of access.

The idea is barely different than a "Singleton" class from client perspective, but if you do ever need another instance, you can just go ahead and create one.

Soma