0

In my project we have a few singletons, which tends to be problematic in unit tests. So I wanted to find solution for the problem. Here is what I came with so far:

smart_singleton.h

class smart_singleton
{
public:
    static std::shared_ptr<smart_singleton> get_instance();

private:
    smart_singleton();

    smart_singleton(const smart_singleton&) = delete;
    smart_singleton operator=(const smart_singleton&) = delete;
    smart_singleton(smart_singleton&&) = default;
    smart_singleton& operator=(smart_singleton&&) = default;

    static std::weak_ptr<smart_singleton> weak_instance;
};

smart_singleton.cpp

std::weak_ptr<smart_singleton> smart_singleton::weak_instance;

std::shared_ptr<smart_singleton> smart_singleton::get_instance()
{
    if (auto existing_instance = weak_instance.lock()) {
        return existing_instance;
    } else {
        std::shared_ptr<smart_singleton> tmp_shared(new smart_singleton());
        weak_instance = tmp_shared;
        return tmp_shared;
    }
}

smart_singleton::smart_singleton()
{

}

The difference is that you need to hold one shared_ptr from "get_instance()" anywhere in your code for the object to not be destroyed. In your prodution code that would be somewhere in the main function (or some object that is alive for the whole scope of main). In UT that would be for the duration of one test.

I would appreciate a feedback what flaws do you find in such a implementation

Marcin K.
  • 683
  • 1
  • 9
  • 20
  • _"In my project we have a few singletons"_ Isn't that contradictorily? – πάντα ῥεῖ Nov 19 '16 at 10:42
  • 3
    for a review you better ask on http://codereview.stackexchange.com/ – 463035818_is_not_an_ai Nov 19 '16 at 10:42
  • 1
    The flaw is that you use the Singleton pattern. Don't. Breaking your tests is just a symptom of the fundamental problem with Singletons, and frankly, your cure looks worse than the disease. – Christian Hackl Nov 19 '16 at 10:43
  • @πάνταῥεῖ i dont think so, just as "few individuals" isnt contradictory :P – 463035818_is_not_an_ai Nov 19 '16 at 10:43
  • πάντα ῥεῖ We have a few classes that have only one instance. @Christian Hackl Can you be more specific? I believe you have a reason to say that, but you haven't shared this reason and this is what im interested in the most. Put aside please whether or not singleton should be avoided. I wanna hear opinion about this particular implementation (made to achieve a goal - independet unit tests) tobi303 Thanks for the link! – Marcin K. Nov 19 '16 at 10:51
  • Creating a new `shared_ptr` for each call looks expensive. And you also have a multi-threading problem where parallel calls to the `else` part might create several instances of the singleton. – Bo Persson Nov 19 '16 at 10:55
  • @MarcinKorn: Well, one of the liabilities of Singleton is precisely that it makes testing harder. That's a concious decision you make when you choose to use the pattern. Pros and cons of Singletons have been discussed a thousand times on Stackoverflow. Just google for "singleton bad stackoverflow" and you will find numerous questions and answers. – Christian Hackl Nov 19 '16 at 10:57
  • @Christian Hackl I think I didn't stress it well enough. I'm interested in feedback regarding the implementation of singleton pattern that I made. Please put aside the problem of singleton patter itself being discouraged – Marcin K. Nov 19 '16 at 11:10
  • @MarcinKorn: Your implementation would not survive my code review. It's confusing and solves the wrong problem. You have also twisted your code for unit tests, which is not a good idea to begin with. For a more detailed review, you should do what tobi303 suggested. – Christian Hackl Nov 19 '16 at 11:19
  • Really good answer was provided by Justin's [post here](http://codereview.stackexchange.com/questions/147506/unit-testing-friendly-singleton) – Marcin K. Nov 22 '16 at 14:09

0 Answers0