0

In my program I'm using one of the most common Singleton pattern used in C++ that I enhanced logically with the arrival of C++11, here is my final code :

template <class T>
class Singleton {
  public:
    ~Singleton() = default;

    static T& getInstance() {
        if (instance.get() == nullptr)
            instance = std::shared_ptr<T>(new T());

        return *instance;
    }

    static void killInstance() {
        instance = std::shared_ptr<T>(nullptr);
    }

  protected:
    Singleton() = default;
    Singleton(const Singleton&) = delete;
    Singleton& operator=(const Singleton&) = delete;

  private:
    static std::shared_ptr<T> instance;
};

template <typename T>
std::shared_ptr<T> Singleton<T>::instance(nullptr);

So all over the internet we can see this pattern taught on many sites, but I have one issue with it, in every single sites I found, it is said that every singleton must be declared this way :

class MySingleton : public Singleton<MySingleton> {
    friend classSingleton<MySingleton>
};

But as far as I remember, the standard (and not only since C++11) clearly states that a class is automatically the friend of all of its children (on the contrary children are not since they it's based on parent's accessibility and inheritance's accessibility), which quite honestly makes sense. So this friend statement I see everywhere really look quite useless AND senseless isn't it? Anyone can confirm I can get rid of it without any issue?

Thanks in advance everyone ;)

Jeremy B.
  • 73
  • 9
  • `shared_ptr` looks like the wrong smart pointer here. Also, do you really need `killinstance`? That will invalidate any references to the instance held by others. I would throw this code away and start from scratch. – juanchopanza Aug 03 '14 at 10:15
  • @juanchopanza what does that have to do with his question? – fonZ Aug 03 '14 at 10:17
  • 1
    @fonZ I am pointing out that this implementation is awful, so it is not worth spending time on it. The question itself has absolutely nothing to do with singletons, so you could ask what the code has to do with the question. – juanchopanza Aug 03 '14 at 10:18
  • As for your actual question, no, a class is not automatically a friend of its children. Where did you read that? – juanchopanza Aug 03 '14 at 10:18
  • @juanchopanza looks perfectly fine to me the implementation. Maybe the names could have been chosen better but besides that it follows the Singleton pattern AND the c++ 11 standard. – fonZ Aug 03 '14 at 10:20
  • @fonZ The fact that it looks fine to you does not mean it is not awful. – juanchopanza Aug 03 '14 at 10:21
  • @juanchopanza this code works and is the syntax is correct. Some people have difficulties to read complicated syntax, especially because they have not been used to the c++11 syntax, but that is not the issue and a personal preference. – fonZ Aug 03 '14 at 10:24
  • @juanchopanza yes sorry comes from an old commit I replaced it with unique_ptr since then, but still this is not awful ;) This is just not the best smart pointer for this case, but it does the trick. The fact unique_ptr is better is only because there will only ever be one owner ! Well on another question on Stack Overflow that I once favorited, but well, since I don't take much care of my favs well... If I find it I'll post it again ! – Jeremy B. Aug 03 '14 at 10:24
  • @fonZ The code is broken. If you call `killinstance()`, which is public, you invalidate all references to the instance. – juanchopanza Aug 03 '14 at 10:26
  • @juanchopanza that is an implementation detail!!! The question is, is the friend needed?! And the answer is, no if you make the necessary functions and attributes protected. – fonZ Aug 03 '14 at 10:27
  • @juanchopanza Well many other more recent discussions indeed state the opposite so well, thanks for pointing out, since I had a wrong knowledge on that (never used it fortunately enough). I guess the answer was probably compiler specific (maybe an older VC++ since older VC++ tended to not follow the standard too well...). Anyway thanks a lot ;) You can post that as answer so I can accept it by the way – Jeremy B. Aug 03 '14 at 10:28
  • @fonZ Stupid me... Indeed I should change the accessibility. – Jeremy B. Aug 03 '14 at 10:29
  • 3
    @fonZ A public method that breaks everything is not an implementation detail. – juanchopanza Aug 03 '14 at 10:30
  • @juanchopanza No nothing is invalidated, the whole point of smart pointers are to handle themselves, when I put it to null, it is deleted automatically, from cppreference.com : std::shared_ptr is a smart pointer that retains shared ownership of an object through a pointer. Several shared_ptr objects may own the same object. The object is destroyed and its memory deallocated when either of the following happens: the last remaining shared_ptr owning the object is destroyed. the last remaining shared_ptr owning the object is assigned another pointer via operator= or reset(). Here I reassign it – Jeremy B. Aug 03 '14 at 10:31
  • 2
    It *is* invalidated, because `getinstance` returns a reference to the managed object. Any code holding that reference will get broken the moment `killinstance` is called. – juanchopanza Aug 03 '14 at 10:32
  • @jaunchopanza if there is a shared pointer left somewhere, the object is not destroyed. So nothing breaks. And if you are holding a reference to a singleton somewhere, then you are doing it wrong anyway. Because a singleton is ment to be called statically. – fonZ Aug 03 '14 at 10:35
  • I never keep the instance in a variable because the whole point of Singletons is this. killInstance is there for whenever I don't need the pointer anymore. Yet I should indeed keep the shared_ptr and return the shared_ptr, this way the caller will maintain the Singleton alive for himself only. Thanks for pointing out ;) – Jeremy B. Aug 03 '14 at 10:36
  • @fonZ That ! It is not meant to be maintained in a variable, though I'll do what I just said for error prevention. – Jeremy B. Aug 03 '14 at 10:37
  • 1
    Singleton is an anti-pattern, a glorified global variable, the most evil thing the otherwise benevolent GoF book introduced in the world of software engineering. The "only one instance is allowed" requirement is almost always artificial or imagined, and as far as the benefits of global access are concerned, there is obviously no difference to a global variable to begin with. In 99% of all Singletons I've ever seen in anyone's code, in any programming language, it would have been sufficient to *just create one instance of the class*, or use static functions, or free-standing ones. – Christian Hackl Aug 03 '14 at 11:04
  • 2
    Additionally, if you really must use a Singleton, then it's usually wrong to derive from it. It's cleaner to make the Singleton final and extract the configurable behaviour to an abstract implementation class. And of course, killing a Singleton goes directly against its intention. – Christian Hackl Aug 03 '14 at 11:06
  • @ChristianHackl I don't see how static classes are less evil, they're one global instance as well. Singletons are only deferring the staticity and are not evil when used [correctly.](http://stackoverflow.com/questions/86582/singleton-how-should-it-be-used?rq=1) It is the best practice when you're supposed to have ONLY one instance (and having several breaks the behavior) and your class is final (never inherited from) ;) Sure we can create only one global accessible instance but a Singleton makes its point clear and prevent multiple instantiation (at best your way of doing Singletons is evil). – Jeremy B. Aug 03 '14 at 11:25
  • @ChristianHackl I didn't mean static classes are evil by the way, they're useful too, in different cases (and I also used them) ;) Just keep in mind that almost nothing is evil as long as it fits the design correctly and allows it to be evolutive, and enforces as much as you can the coding practices you want to put in place in your project ! – Jeremy B. Aug 03 '14 at 11:30
  • 1
    @DevilBlackDeath: Singletons add unnecessary complexity over a basic language feature (creating an instance of a class), they add synchronization troubles (the recommendation is usually to call `getInstance` in `main` before threading starts), and they create dependencies throughout your entire code base, hidden inside of the implementation. And as I said, you will hardly ever encounter a class of which no two instances may conceptually exist. In fact, allowing only one instance is about the exact opposite idea of a "class" to begin with, a complete contradiction in itself. – Christian Hackl Aug 03 '14 at 11:40
  • @ChristianHackl Yeah but this is mostly for organization, sure we could do it without classes, but it would be much more hard to read. Then again, I don't see how static classes fit more in the OO programming =/ Plus the synchronization trouble is valid for just about every classes, Singletons, statics, and normal ones, it is up to the developer to resolve syncing issues with mutexes and other threading tool ;) – Jeremy B. Aug 03 '14 at 12:54
  • @DevilBlackDeath: "Fitting more in the OO programming" is not a virtue by itself, especially in C++. If you don't need run-time choice of an abstract operation's implementation, you shouldn't use OOP. Also, I am not saying that every Singleton should be replaced by a class with static functions. It's just one possible simplification. The most common replacement would be just creating one instance and passing a reference to it around, and generally designing classes such that additional instances don't hurt. – Christian Hackl Aug 03 '14 at 14:08
  • In my case, I am programming a game engine, and game design requires (even in a lot of articles by professionals talking about game architecture) that a lot of informations need to be passed around in only one instance, and I don't want my methods signatures to become over complicated ;) Plus I still don't see how Singletons go agains OO programming. The Titanic (object) was unique, but still it had a construction plan (class) ! Plus if it really is not OO programming, solve that issue in Java where there are ONLY classes and is supposed to more OO (according to its users) :) – Jeremy B. Aug 03 '14 at 14:58

1 Answers1

2

But as far as I remember, the standard (and not only since C++11) clearly states that a class is automatically the friend of all of its children

This is a misconception. The concepts are orthogonal: a class is not a friend of its children.

juanchopanza
  • 223,364
  • 34
  • 402
  • 480
  • So, somebody out there must think that base classes are friends of their children, given that this answer has been down-voted :-) – juanchopanza Aug 03 '14 at 10:47
  • Yes... Well even though I used to think that, I would never have down voted without reading it with my own eyes in the official standard :P Well at least there's no rep down for down votes, but now people might think your answer is wrong when it's actually not... – Jeremy B. Aug 03 '14 at 11:27