[RESOLVED] Using boost's singleton class

[Mybowlcut]

I'm trying to save myself some time and use boost's singleton class instead of writing my own... but it's giving me some weird error that I can't resolve:

#include <iostream>
#include <string>
#include <map>
#include <exception>

#include "boost/shared_ptr.hpp"
#include "boost/pool/detail/singleton.hpp"

class Surface
{
public:
  Surface()
  {
    std::cout << "Loaded a surface." << std::endl;
  }
};

class Surface_Cache : public boost::details::pool::singleton_default<Surface_Cache>
{
public:
  typedef boost::shared_ptr<Surface> Surface_Ptr;
  typedef std::map<const std::string, Surface_Ptr> surface_cache;
  typedef surface_cache::iterator cache_it;

  Surface_Ptr Load_Surface(const std::string& file_name)
  {
    cache_it it = cache.find(file_name);

    if(it == cache.end())
    { // Add surface.
      Surface_Ptr s(new Surface());
      cache.insert(std::make_pair(file_name, s));
      return s;
    }
    return it->second;
  }
private:
  surface_cache cache;
};

int main()
{
  Surface_Cache::instance().Load_Surface("example.png");

  return 0;
}

1>c:\documents and settings\bill\my documents\visual studio 2005\projects\test\test\main.cpp(39) : error C2248: 'boost::details::pool::singleton_default<T>::singleton_default' : cannot access private member declared in class 'boost::details::pool::singleton_default<T>'
1>        with
1>        [
1>            T=Surface_Cache
1>        ]
1>        e:\everything\programs\boost\boost\pool\detail\singleton.hpp(71) : see declaration of 'boost::details::pool::singleton_default<T>::singleton_default'
1>        with
1>        [
1>            T=Surface_Cache
1>        ]
1>        This diagnostic occurred in the compiler generated function 'Surface_Cache::Surface_Cache(void)'

Why is it complaining?

Cheers.

[Lindley]

Don't know boost well enough to say, but I'm not sure this is actually saving you time if it's giving you trouble. Making a class a singleton is only about 5 extra lines of code, after all.

[Mybowlcut]

Don't know boost well enough to say, but I'm not sure this is actually saving you time if it's giving you trouble. Making a class a singleton is only about 5 extra lines of code, after all.

5 extra lines? Do you have an example to back that up? Haha. I thought you had to write a Singleton class and then derive your class from it?

Edit: I did it haha. It wasn't that hard after all.

#include <iostream>
#include <string>
#include <map>
#include <exception>

#include "boost/shared_ptr.hpp"

template<typename T>
class Singleton
{
public:
  virtual ~Singleton() {}

  static T& Instance()
  {
    static T object;
    return object;
  }
};


class Surface
{
public:
  Surface()
  {
    std::cout << "Loaded a surface." << std::endl;
  }
};

class Surface_Cache : public Singleton<Surface_Cache>
{
public:
  typedef boost::shared_ptr<Surface> Surface_Ptr;
  typedef std::map<const std::string, Surface_Ptr> surface_cache;
  typedef surface_cache::iterator cache_it;

  Surface_Ptr Load_Surface(const std::string& file_name)
  {
    cache_it it = cache.find(file_name);

    if(it == cache.end())
    { // Add surface.
      Surface_Ptr s(new Surface());
      cache.insert(std::make_pair(file_name, s));
      return s;
    }
    return it->second;
  }
private:
  surface_cache cache;
};

int main()
{
  Surface_Cache::Instance().Load_Surface("example.png");

  return 0;
}

[Lindley]

5 extra lines? Do you have an example to back that up? Haha. I thought you had to write a Singleton class and then derive your class from it?

Edit: I did it haha. It wasn't that hard after all.

Well, you *can*, but there's no real need to. Making a class a singleton requires:
-Marking all constructors and operator= private. Even if you don't implement operator= or the copy constructor, they should be declared private to avoid auto-generating them public.
-Writing a getInstance() function.
-As far as I know, if you use the version of getInstance() with a static object (as you have), there isn't even a need for a mutex to ensure thread-safety. If you were going to use dynamic allocation instead, there would be.

[GNiewerth]

It´s not that easy

You have to make sure your singleton object cannot be copied, therefore you have to make the copy constructor/assignment operator private. Or derive from boost::non_copyable.

[Mybowlcut]

Wait wait wait... so I make constructors, copy constructor and assignment operator private on the Singleton class? Or every class that derives from it? I'm assuming the Singleton haha.

[Lindley]

There's no need to make a Singleton class that others derive from at all. It's equally easy to just turn any old class into a singleton.

[Mybowlcut]

There's no need to make a Singleton class that others derive from at all. It's equally easy to just turn any old class into a singleton.

Well instead of doing it several times I thought it'd be easier to do it once and then just derive from that? But just out of curiousity, how would you turn a class into a singleton?

[laasunde]

How about

class Settings 
{
  public:
      static Settings * Instance();
  protected:
      Settings ();
      Settings (const Settings &);
      Settings & operator= (const Settings &);
  private:
      static Settings * pinstance;
};

[Lindley]

I just explained it up-thread.

The problem with deriving from a singleton class is that you've still got to mark your own class uncopyable. I don't think simply declaring the base class uncopyable is good enough; you'd probably still have to declare your constructors private to avoid a compiler error (so you'd get enforcement, but no real effort savings), and I'm not sure operator= would be caught at all.

[GNiewerth]

All you have to do is to implement these lines of code:

class MyClass
{
public:
   static MyClass& instance()
   {
      static MyClass theInstance;

      return theInstance;
   }

private:
   MyClass(); // <-- must be implemented, of course

   // these need to be declared, but don´t need to be implemented
   MyClass( const MyClass& op ); 
   MyClass& operator=( const MyClass& op ); 
};

Edit:
Added the static modifier for instance() function

[exterminator]

Why are you deriving from boost::details::pool::singleton_default? It is a simple template on the type parameter for which you should pass the type which you want to make a singleton. That is, just define your class in a normal way and use it as:

A & ref = boost::details::pool::singleton_default<A>::instance();

A being your class. However, there is a limitation to this template. It needs to have no throw default constructor for A. Now, since you would provide a default constructor and that too a public one, singleton-ness of that class cannot be mandated. You might include some logic inside your class' constructor to throw on subsequent constructor calls.

There could be another way - defining singleton_default<T> as a friend of T. That is, make:

boost::details::pool::singleton_default<A>

as a friend of A. Something like this:

#include <boost/pool/detail/singleton.hpp>
namespace bdp = boost::details::pool;
struct A{
	friend class bdp::singleton_default<A>;
private:
	A(){}
	~A(){}
public:
	void f(){ std::cout << "hello\n";}
};
int main(){
	A & ref = bdp::singleton_default<A>::instance();
	ref.f();
}

Don't know if it works though.

[exterminator]

As the documentation on boost says that this might be replaced soon by a boost singleton library, it might be better to wrap the singleton getInstance inside a function to localize the effect of that change on your codebase. In fact, I have seen applications that localize any usage of a 3rd party lib. Your call depending upon the effort needed.

[Mybowlcut]

Why are you deriving from boost:etails:: pool::singleton_default?

When there is no example usage on the internet (not even on the boost documentation page) and then you factor in my level of expertise in C++ you can see why I'd try and derive from it. After reading your post I realised I don't want to touch singleton_default haha.

I just explained it up-thread.

The problem with deriving from a singleton class is that you've still got to mark your own class uncopyable. I don't think simply declaring the base class uncopyable is good enough; you'd probably still have to declare your constructors private to avoid a compiler error (so you'd get enforcement, but no real effort savings), and I'm not sure operator= would be caught at all.

Sorry Lindley, I should have read that.

All you have to do is to implement these lines of code:

class MyClass
{
public:
   MyClass& instance()
   {
      static MyClass theInstance;

      return theInstance;
   }

private:
   MyClass(); // <-- must be implemented, of course

   // these need to be declared, but don´t need to be implemented
   MyClass( const MyClass& op ); 
   MyClass& operator=( const MyClass& op ); 
};

So then you'd put functions that need to be called by code in the public section? I thought that someone would mistakingly try and call them without going through instance but then I realised that the object can't even be constructed so that won't happen haha.

Cheers for all your help!

[exterminator]

When there is no example usage on the internet (not even on the boost documentation page) and then you factor in my level of expertise in C++ you can see why I'd try and derive from it.

You searched at all the right places except one. Considering the singleton_pool is part of the pool library, and not as an independent lib on its own, you should have looked inside boost/pool for example usage. And in fact it is being used there: singleton_pool.hpp.

After reading your post I realised I don't want to touch singleton_default haha.

Just to clarify, I did not recommend against using it. I just provided guidelines around something that might change/or be replaced in the future.