On use of C++ singleton class

Question

I have looked at this thread on singleton class implementation, but not clear on how to use it in practice. To make the context more concrete, say I have a input stream std::istream instance that many different class need to access, but instead of passing it in for each class constructor, I am thinking of using a singleton class Connection to wrap this info. So a client can just call:

Connection.getInstance().get_input_stream();

My questions are two fold: (1) is this a proper use of singleton class (2) on implementing this, I have tried something like this:

class Connection {
public:
    static Connection& getInstance() {
        static Connection instance; // Guaranteed to be destroyed   
        return instance;
    }
    std::istream& get_istream() {
        return is;
    }

    void set_istream(std::istream & stream) {
            is = stream;
    }
private:
    std::istream& is;
}

First this code doesn't compile for some reason. Second it feels awkward that you have to call set_istream() before it is usable. Is there a better way to get this done? Thanks.

EDIT: Apparently, trying to assign a reference is my silly doing, as many of you pointed out. The second part is how to pass in stream information into singleton - it seems not worth it, which suggests this might be a wrong case for using it. thanks you all for your answers.

It doesn't compile because you're trying to assign to a reference variable. You can only initialize it in constructor's initializer list. — jrok, Feb 16 '12 at 18:26
possible duplicate of [What is so bad about Singletons?](http://stackoverflow.com/questions/137975/what-is-so-bad-about-singletons) — Mike Seymour, Feb 16 '12 at 18:26
Also to use it you need to use the static method accessor like Connection::getInstance().get_input_stream(); — Adrian Cornish, Feb 16 '12 at 18:28
@MikeSeymour: Though very relevant, not even close to duplicate. — Mooing Duck, Feb 16 '12 at 18:30
@MooingDuck: It answers the first question; the remaining questions are only relevant if the OP decides to ignore it. — Mike Seymour, Feb 16 '12 at 18:36
Look at This also!! http://stackoverflow.com/q/8906817/1155650 — Rohit Vipin Mathews, Feb 24 '12 at 10:54

Jerry Coffin · Accepted Answer · 2012-02-16T18:42:51.297

3

A reference can't be modified after creation, so to "set" the istream, you'd have to pass it to the ctor:

class Connection {

    /* ... */

    Connection(std::istream & stream) : is(stream) {}
private:
    std::istream& is;
}

That raises a problem, however, of how you pass the correct stream to the ctor when you're creating a static instance -- and the answer to that is that it's non-trivial (at best). You could use a pointer instead of a reference, so you can create an object and point it at an actual stream later, but at that point you're basically asking for trouble (requiring two-stage initialization).

Ultimately, it's just not worth it. My advice would be against using singleton in this case at all. It looks like it's adding quite a bit of work, and providing little (if anything) in return.

Quite a few people are starting to advise against using singletons at all, at least in C++, and quite frankly, I tend to agree most of the time. This kind of situation is why -- you usually end up with little in return for the work you do on it.

edited Feb 16 '12 at 18:42

answered Feb 16 '12 at 18:31

Jerry Coffin

437,173
71
570
1,035

1

Your code won't work - you're not passing a parameter to the constructor in getInstance. – obmarg Feb 16 '12 at 18:33
I got the point of avoiding singleton - one question on the public Constructor() though, is it still a singleton class when Connection() is called more than once? – Oliver Feb 16 '12 at 18:52
@Oliver: Actually, you'd normally make the ctor private (which is how I've shown it, though private by default rather than explicitly specified). A singleton almost always needs the ctor private so the static `GetInstance` (or whatever) is the only way to create one. – Jerry Coffin Feb 16 '12 at 19:01

Mooing Duck · Answer 2 · 2012-02-16T18:36:04.250

I've always been told to avoid singletons, so there's two parts to this:

class Connection {
public:
    static std::istream& getInstance() {
        assert(is);
        return *is;
    }    
    static void set_istream(std::istream & stream) {
            is = &stream;
    }
private:
    static std::istream* is;
}
std::istream* Connection::is = NULL;

int main() {
    Connection::set_istream(std::cin);
    Connection::getInstance() << "moomoomoo\n";
}

Note I made the stream static, a pointer, and global (so it's actually a singleton. Your code.... wasn't. There's no reason to have that many references laying around, and if you have is = blah where is is a reference, that does not reseat the reference. It would copy the right to the left, which for streams, doesn't make sense and wouldn't even compile.

Of course, singletons are bad. Use globals, or non-singletons. (This is a global, which is only creatable once)

//params are ignored after first call
template<typename... Ts>
std::istream& Connection(Ts... Us) { 
    static std::ifstream c(std::forward<Ts>(Us)...);
    return c;
}

int main() {
   //construct and use
   Connect("input.txt") << "thing" << std::endl;
   Connect() << "stuff";
   Connect("this is ignored") << "IM A COW STOP SAYING IM A DUCK";
}

score 1 · Answer 3 · answered Feb 16 '12 at 18:48

It doesn't compile for two reasons. First you have a very minor problem with the calling syntax. Fix it like this:

Connection::getInstance().get_input_stream();

Second is that you can't assign to a reference, it must be initialized at construction time. Since your constructor is called from the middle of getInstance, that's impractical. Use a pointer instead.

Yes it's awkward to need to call set_istream before your singleton is usable, but that seems unavoidable.

On use of C++ singleton class

3 Answers3