0

Let's assume I have a singleton class:

class Singleton {
public:
    static Singleton* getInstance();
    void doit();
    std::vector<Object*>& getVector();

private:
    std::vector<Object*> _vector;
    static Singleton *instance;
    Singleton();
    ~Singleton();
    Singleton(const Singleton&);
};

class Delegator {
public:
        void main();
}
  • In the doit method I populate the _vector with pointers to objects.
  • In the main() from the Delegator class, I call getVector() and display the results.

Given this abstraction, I have the following questions:

  1. Can I delete all the pointers to instances of Object in the main() from Delegator (after displaying the results). If yes, is this recommended?
  2. Does the singleton ever get destroyed? In other words, will the reference returned in getVector() always be valid?
  3. I return a reference to a vector instead of a copy of the vector in getVector(). Given that the vector only contains pointers to objects and will not modify the vector content outside the Singleton class, is there any efficiency gained from returning a reference?

Thank you in advance

  • There is no actual implementation in there! How can we know if anything will be valid? – R. Martinho Fernandes Jan 02 '13 at 14:30
  • 7
    `getInstance` returns a copy of the singleton? That's not much of a singleton, then. – jogojapan Jan 02 '13 at 14:32
  • 5
    "Let's assume I have a singleton" there's your problem – thecoshman Jan 02 '13 at 14:33
  • Isn't (1.) and (3.) a contradiction? In (1.) you say you'll delete the elements of the vector from `main`, but in (3.) you say you assume the vector contents will never be modified from outside the `Singleton` class. – jogojapan Jan 02 '13 at 14:37
  • I have no implementation yet, I did a quick structure description, the questions are meant to be purely theoretical. – Constantin Levodeanschi Jan 02 '13 at 14:38
  • Could you explain what the objects are you store in the vector? And why a singleton is used for this? And why the singleton adds to the vector, but anyone can delete from it? Perhaps there are good reasons, but at first glance, it seems to contradict both the idea of a singleton pattern as well as that of encapsulation (and therefore makes me wonder why in every other aspect you base the code on those ideas). – jogojapan Jan 02 '13 at 14:43
  • 2
    You might also find this SO post useful: http://stackoverflow.com/questions/1008019/c-singleton-design-pattern – jogojapan Jan 02 '13 at 14:44
  • Singleton is just a global variable with a fancy name gussing it up so you can fool yourself about the fact that you're using a global variable. I would go so far as to say that they shouldn't be used in any program. – Omnifarious Jan 02 '13 at 15:02
  • 1
    @jogojapan Thank you for the link, I was exactly what I was looking for. As for your previous questions, I wasn't really a requirement to make the class singleton, but just thought it might be required in a later phase. – Constantin Levodeanschi Jan 02 '13 at 15:23

3 Answers3

4

A typically singleton pattern looks like this (other variations are possible, like using reference instead of pointer for the instance).

singleton.h:

class Singleton {
public:
    static Singleton* getInstance();
    ...

private:
    static Singleton *instance;
    Singleton();                 //permit construction of instances
    ~Singleton();                //permit deletion of instances
    Singleton(const Singleton&); //don't provide an implementation for copy-ctr.
};

singleton.cpp:

Singleton *Singleton::instance = 0;

Singleton::Singleton() {
    // init your stuff
}

Singleton::~Singleton() {
}

Singleton *Singleton::getInstance() {
    if(!instance) instance = new Singleton();
    return instance;
}

In your case, you return a copy of a singleton instance, which violates the singleton pattern.

To answer your 2. question: A singleton should never be deleted. However, we define a destructor so we can make it private, otherwise the caller can delete Singleton::getInstance() which is not a good idea.

That being said, singletons are in most cases considered an anti-pattern. In very most cases a utility class is better suited. Utility classes are a very similar concept, but they implement everything static instead of using an instance. When you need initialization code, do this with an init() method (since no constructor involved).

leemes
  • 44,967
  • 21
  • 135
  • 183
  • *Utility classes are a very similar concept, but they implement everything static instead of using an instance.* Well just make it a namespace then. No need for a class. –  Jan 02 '13 at 14:47
  • `Singleton` itself doesn't imply lifetime though in most usecases it will be coupled with it. – SomeWittyUsername Jan 02 '13 at 14:51
  • @Zoidberg'-- Good point. I'm just used to write them as classes. Seems to be remains from Java programming course in university... -.- So is my code for this Singleton pattern written in a Java-style ;) – leemes Jan 02 '13 at 14:51
  • @icepack What exactly do you mean? Is something wrong with my class? How can I improve it? – leemes Jan 02 '13 at 14:52
  • No, there is nothing wrong with your class. I mean that singleton doesn't have to be valid always. It's possible to have a singleton instantiated only during a certain period of program runtime if it fits the requirements. – SomeWittyUsername Jan 02 '13 at 14:56
  • @icepack Ah, that's what you meant. Ok, good point. However, we can think of a couple of variants for singletons. In a project 2 years ago, we have even been about to introduce a "multiton" class, which existed exactly once per provided integer: `getInstance(int foo)` - But we decided against it. – leemes Jan 02 '13 at 15:05
2
  1. That totally depends on your requirements and design. Your presented code is neutral from this perspective.

  2. Again, that's up to your design and implementation. The singleton pattern itself doesn't imply lifetime. If you need the singleton to be valid for a certain lifetime, you need to ensure that by other means (e.g., define a global instance of it).

  3. Yes, there is a performance gain. Instance of vector is not a simple array and contains a substantial amount of data besides the actual stored information. All the data will be copied if returned by value (put aside the optimizations for this discussion). Also, as was correctly noted in another answer, returning by value actually breaks the singleton pattern.

SomeWittyUsername
  • 18,025
  • 3
  • 42
  • 85
2

My first answer to the question you didn't ask is this:

Do not use Singleton. It's an anti-pattern. It makes the code it touches worse. It kills testability and makes it harder to modify your program in the future.

This really excellent article talks in great detail about why Singleton is a bad idea.

To answer the questions you did ask...

  1. Yes, you can. Of course, those pointers will still be in the vector so the vector will now contain a ton of dangling pointers. So you ought to clear out the vector too.
  2. With the code you have it will never be destroyed. By some definitions that makes your Singleton a memory leak. So yes, the reference will always be valid.
  3. Yes, there is a huge performance improvement to returning a reference. Copying a vector means making a copy of all the elements in the vector. Note that this means all the pointers will be copied, not the Objects they point to. Additionally, if you return a copy and have the main function delete all the pointers you are absolutely guaranteed to have a vector lying around with a whole ton of dangling pointers in it.
Omnifarious
  • 54,333
  • 19
  • 131
  • 194
  • Yes, I didn't really think it through, I could write my code in a more elegant way without the singleton indeed. The singleton wasn't really a requirement, but for some reason I thought it should be there. – Constantin Levodeanschi Jan 02 '13 at 15:20