17

I've been wondering how to pass argument to a singleton contructor. I already know how to do a singleton, but I've been unlucky to find a way to do it.

Here is my code (part of it).

Questionnary* Questionnary::getInstance(){

    static Questionnary *questionnary = NULL;

    if(questionnary == NULL){
        cout << "Object created";
        questionnary = new Questionnary();

    }
    else if(questionnary != NULL){
        cout << "Object exist";
    }

    return questionnary;
}

Questionnary::Questionnary(){
    cout << "I am an object";
}

//This is want i want to acheive
Questionnary::Questionnary(string name){
    cout << "My name is << name;
}

Many thanks in advance

(BTW i know how and why a singleton is bad)

Chax
  • 1,041
  • 2
  • 14
  • 36
  • 1
    Since you already have a singleton, there's no harm in creating a few global variables, one for each constructor argument, then assign values to all these variables before calling `getInstance()` the first time. And there's no need to `new` singleton instance, just make it a static variable within `getInstance()`, C++11 even guarantees that its initialization will be thread safe. – Praetorian Jan 28 '14 at 02:12

5 Answers5

16

Let me extend Martin York's answer for your use case. I recommend using pointer for argument(s) in this particular situation, as we make use of its inherent property, that it can be "empty".

class Questionnary
{
  std::string _str;

  static Questionnary& getInstanceImpl(std::string* const s = nullptr)
  {
    static Questionnary instance{ s };
    return instance;
  }

  Questionnary(std::string* const s)
    : _str{ s ? move(*s) : std::string{} } // employ move ctor
  {
    if (nullptr == s)
      throw std::runtime_error{ "Questionnary not initialized" };
  }

public:
  static Questionnary& getInstance()
  {
    return getInstanceImpl();
  }
  static void init(std::string s) // enable moving in
  {
    getInstanceImpl(&s);
  }

  Questionnary(Questionnary const&) = delete;
  void operator=(Questionnary const&) = delete;
};

I find this approach less confusing, as it let's you get the instance after first initialization without (anyway discarded) arguments:

// first init
Questionnary::init("my single Questionnary");

// later on ...
Questionnary& q = Questionnary::getInstance();

Edit: Removed argument from getInstance function and a few optimizations.

neonxc
  • 802
  • 9
  • 21
  • 1
    its nicer than accepted answer in that you dont have to pass a parameter, but I still dont like that you can pass a parameter (that will be ignored in case the instance exists already). – 463035818_is_not_an_ai Sep 13 '18 at 13:00
  • @user463035818 I edited the code not to distract user with unused parameter in the `getInstance` function. Is this acceptible shape for you? – neonxc Oct 28 '18 at 12:13
  • 1
    Nice solution but I think the first call should be `Questionnary::init(s)` with your edit. – OSE Nov 07 '18 at 04:01
  • @neonxc Would you mind explaining the use of the move constructor in the Questionnary constructor? I'm still trying to get to grips with move stuff! Thanks. – cosimo193 Jun 15 '23 at 16:35
11

You don't need to allocate the instance of singleton dynamically. It could look the following way (this is sometimes called "lazy loading singleton" ~ the instance is created late & kinda "automatically"):

#include <iostream>
#include <string>

class Questionnary
{
private:
    // constructor taking string:
    Questionnary(const std::string& name) : name_(name) { }
public:
    static Questionnary& getInstance(const std::string& name)
    {
        static Questionnary q(name);
        std::cout << "My name is: " << q.name_ << std::endl;
        return q;
    }
private:
    std::string name_;
};

int main() {
    Questionnary::getInstance("Josh");
    Questionnary::getInstance("Harry");
}

output:

My name is: Josh
My name is: Josh

Note that constructor will be called only once right when the getInstance is called for the first time.

LihO
  • 41,190
  • 11
  • 99
  • 167
  • Not dynamically allocating the singleton is the right idea, but if you're going to assign the data member directly anyway, what's point of having the constructor argument? – Praetorian Jan 28 '14 at 02:09
  • @Praetorian: That was meant to point out that passed string might be used to change the state of that singleton within a next `getInstance` call as well, but you're right, it makes the constructor useless here. – LihO Jan 28 '14 at 02:11
  • Thanks, so if i get it right, all i need to pass my argument is just to put them in the getInstance function and then pass it to my constructor. – Chax Jan 28 '14 at 02:15
  • 5
    How do I add a getInstance method without parameters to this implementation? (just to get the instance later on, without needing to pass parameters) – Ken Vernaillen Mar 24 '17 at 14:25
  • 6
    I dont understand why this is the accepted answer, as it rather demonstrates how parameters can **not** be passed to the constructor of the singleton. Moreover requiring to pass the argument even when it will be igonored for all but the first call is a major source of confusion – 463035818_is_not_an_ai Jan 29 '18 at 17:15
  • @user463035818 and Ken, please look at my [answer](https://stackoverflow.com/a/52308483/7949231) (perhaps) meeting your requirements – neonxc Sep 13 '18 at 11:57
3

Have a method to create the instance to pass arguments to the constructor and you could assert in the getInstance() method if CreateInstance has not been called prior to calling it. Like:

class Questionnary
{
private:
    // constructor taking string:
    Questionnary(const std::string& name) : name_(name) 
    {
        std::cout << "My name is: " << q.name_ << std::endl; 
    }

    static Questionnary* m_instance;
public:
    static void createInstance(const std::string& name)
    {
        assert(!m_instance);
        m_instance = new Questionary(name);
    }

    static void destroyInstance()
    {
        assert(m_instance);
        delete m_instance;
    }

    static Questionnary* Questionnary::getInstance()
    {
        assert(m_instance);
        return m_instance;
    }
private:
    std::string name_;
};
Eric Fortin
  • 7,533
  • 2
  • 25
  • 33
  • 1
    Nice, but it gets messy if the process of construction requires creation of objects that in themselves refer to `getInstance()`... but then I guess we shouldn't be doing that ;-) For the meantime, I use a `bool` to assert, and I set this _before_ calling `new`, so that the `assert` succeeds _during_ construction. (And just to make it all even uglier, I use placement `new`... I'm a monster) – underscore_d Apr 08 '16 at 11:27
2
//! @file singleton.h
//!
//! @brief Variadic template to make a singleton out of an ordinary type.
//!
//! This template makes a singleton out of a type without a default
//! constructor.

#ifndef SINGLETON_H
#define SINGLETON_H

#include <stdexcept>

template <typename C, typename ...Args>
class singleton
{
private:
  singleton() = default;
  static C* m_instance;

public:
  singleton(const singleton&) = delete;
  singleton& operator=(const singleton&) = delete;
  singleton(singleton&&) = delete;
  singleton& operator=(singleton&&) = delete;

  ~singleton()
  {
    delete m_instance;
    m_instance = nullptr;
  }

  static C& create(Args...args)
  {
    if (m_instance != nullptr)
      {
    delete m_instance;
    m_instance = nullptr;
      }
    m_instance = new C(args...);
    return *m_instance;
  }

  static C& instance()
  {
    if (m_instance == nullptr)
      throw std::logic_error(
        "singleton<>::create(...) must precede singleton<>::instance()");
    return *m_instance;
  }
};

template <typename C, typename ...Args>
C* singleton<C, Args...>::m_instance = nullptr;

#endif // SINGLETON_H

and:

void
singleton_utest::test()
{
  try
    {
      singleton<int, int>::instance();
      UTEST_CHECK(false);
    }
  catch (std::logic_error& e)
    {
      UTEST_CHECK(true);
    }

  try
    {
      UTEST_CHECK((singleton<int, int>::create(1) == 1));
      UTEST_CHECK((singleton<int, int>::instance() == 1));
    }
  catch (...)
    {
      UTEST_CHECK(false);      
    }

  using stester0 = singleton<tester0>;

  try
    {
      stester0::instance();
      UTEST_CHECK(false);
    }
  catch (std::logic_error& e)
    {
      UTEST_CHECK(true);
    }

  try
    {
      UTEST_CHECK((stester0::create().result() == 0));
      UTEST_CHECK((stester0::instance().result() == 0));
    }
  catch (...)
    {
      UTEST_CHECK(false);      
    }

  using stester1 = singleton<tester1, int>;

  try
    {
      stester1::instance();
      UTEST_CHECK(false);
    }
  catch (std::logic_error& e)
    {
      UTEST_CHECK(true);
    }

  try
    {
      UTEST_CHECK((stester1::create(1).result() == 1));
      UTEST_CHECK((stester1::instance().result() == 1));
    }
  catch (...)
    {
      UTEST_CHECK(false);      
    }

  using stester2 = singleton<tester2, int, int>;

  try
    {
      stester2::instance();
      UTEST_CHECK(false);
    }
  catch (std::logic_error& e)
    {
      UTEST_CHECK(true);
    }

  try
    {
      UTEST_CHECK((stester2::create(1, 2).result() == 3));
      UTEST_CHECK((stester2::instance().result() == 3));
    }
  catch (...)
    {
      UTEST_CHECK(false);      
    }
}
1

My version using Modern C++ that wraps an existing type:

#ifndef SINGLETON_H
#define SINGLETON_H

template <typename C, typename ...Args>
class singleton
{
private:
  singleton() = default;
  static C* m_instance;

public:
  ~singleton()
  {
    delete m_instance;
    m_instance = nullptr;
  }
  static C& instance(Args...args)
  {
    if (m_instance == nullptr)
      m_instance = new C(args...);
    return *m_instance;
  }
};

template <typename C, typename ...Args>
C* singleton<C, Args...>::m_instance = nullptr;

#endif // SINGLETON_H

Here is what in looks like in a unit test:

  int &i = singleton<int, int>::instance(1);
  UTEST_CHECK(i == 1);

  tester1& t1 = singleton<tester1, int>::instance(1);
  UTEST_CHECK(t1.result() == 1);

  tester2& t2 = singleton<tester2, int, int>::instance(1, 2);
  UTEST_CHECK(t2.result() == 3);

The problem with this is that instance() requires arguments on each call but only uses them on the first (as noted above). Default arguments cannot be used generally. It may be better to use a create(Args...) method which must precede the call of instance() or throw an exception.

  • Interesting. Also I like how you manage to mention Unit Testing in your answer. It is an aspect that is often forgotten. – Chax Jan 09 '19 at 19:06
  • Maybe I'm being ignorant here, but how is it possible to access a member variable from whithin a static method? Is this a feature of more modern C++ standards? – David Wright Oct 23 '19 at 08:49
  • I am not accessing a member variable. It is mislabeled. I should have labeled it s_instance – amightywind Nov 13 '19 at 21:34
  • Using `delete` explicitly makes the code less modern, actually(( – MasterAler Feb 17 '23 at 00:26