tags:

views:

216

answers:

5

I have a random question about const correctness.

Lets say i have a class that is a singleton.

class Foo : public Singleton<Foo>
{
    friend class Singleton<Foo>;

public:
    std::wstring GetOrSet(const int id) const;

private:
    Foo();
    ~Foo();
    void LoadStringIntoMap(const int id, const std::wstring &msg);

    std::map<int, std::wstring> strMap;
};

The functions are defined as such

std::wstring Foo::GetOrSet(const int stringId) const
{
    if ( strMap.find(stringId) == strMap.end() )
    {
        Foo::GetInstance()->LoadStringIntoMap(stringId, std::wstring(L"HELLO WORLD222"));
    }
    std::map<int, std::wstring>::const_iterator retStr = strMap.find(stringId);
    return retStr->second;
}

void Foo::LoadStringIntoMap(const int stringId, const std::wstring &msg)
{    
    strMap.insert(std::pair<int, std::wstring>(stringId, msg));
}

If i directly get call LoadStringIntoMap i get an error that it cannot convert this pointer from const Foo to Foo &. Which makes sense since your calling a non const function from within a const function. But why is this not an issue when calling the singleton, and doing modification through that.

Is this just really unsafe?

This is what singleton is defined as:

   template <typename T> class Singleton
{
protected:
    Singleton () {};
    ~Singleton () {};

public:
    static T *GetInstance()
    {
        return (static_cast<T*> (m_This));
    }

    static void Destroy()
    {
        if(m_This != NULL)
            delete m_This;
    }

    static void CreateInstance()
    {
        m_This = GetInstance();

        if (m_This == NULL)
        {
           m_This = new T;
        }
    }

private:
    // Unique instance
    static T *m_This;
};

template <typename T> T *Singleton<T>::m_This = NULL;
A: 

Yes, it's just unsafe.

The tools don't have any (straightforward) way of knowing that the 'const' on Foo::GetOrSet is going to apply to the same instance of Foo as Foo::GetInstance().

'Singleton' isn't a policy which means anything to the compiler.

Will Dean
+3  A: 

GetInstance() returns a non-const pointer. As the function GetInstance() is not bound to the object itself, but class-wide, it may be called from a const function.

Essentially, you have tricked yourself out of the const environment; but then, you could do that from any context/state of your program (privateness of members is not bound to specific objects, only classes). In this scenario, you have to care for a safe usage of the singleton accessors on your own.

ypnos
Thanks that makes sense, i put up the code for singleton. I didnt write this code, i am trying to understand it.
UberJumper
A: 

Because compiler cares only about access to this. It doesn't go as far as checking all the paths that can change your object. So yeah, it works. Is it unsafe? It depends :) Shouldn't be in most cases but sure there's a set of circumstances that can make it all fall over.

vava
+1  A: 

Its not that its unsafe its just totally illogical. If you don't want to have a constant this pointer than don't specify the function as const. Its that simple. The compiler can't do much optimisation there anyway.

The reason it works is because the singleton passes a new this pointer NOT inherited from the parent function. It doesn't matter that they are both the same address .. one is const which the function can't use. One isn't

Goz
+1  A: 

The code illustrates that it doesn't make a lot of sense to have a const pointer to a singleton, when you can grab a non-const reference to the singleton at any time, via the global getter. The global getter says, "sure, anyone at all can modify the object any time they like. That's fine". The const member function says, "oh, no, it'd be really bad to modify this object".

If the class wasn't singleton, so that it wasn't certain that this refers to the same object that GetInstance returns, then this might make perfect sense in a lot of cases. Since they do refer to the same object, and everyone knows they do because it's singleton, they can't both be right.

You can't ever assume that some globally-accessible object won't be modified (at least, you can't assume the const system will help you avoid modifying it - there may be some other properties of the class which make guarantees about modifications). So either GetInstance is unsafe, or the motivations of the person calling member functions on a const Foo are unsafe, depending on who is actually correct as to whether the object should be modified in the given bit of code. In this case, I'm not sure what the point is of defining any const member functions of an object which is globally modifiable - if someone has a const reference to it, then where did they get that reference from, and why? And what makes anyone think a function called "Get**OrSet**" should be const in the first place?

One option is for GetInstance to return const Foo&, but have some mutable member variables for the things which it's OK for anyone to modify, any time, even through a const reference. But it depends what Foo is actually for: mutable should really only be used for things like caches which don't modify the "observable" guaranteed behaviour of the object.

Steve Jessop