Monitoring errors with singleton

Question

I need to monitor my functions for "errors" and want to print the warnings at the end of the functions. I thought using singleton class could be a good idea in here (code inspired by this example):

class Warning
{
 bool status;
 std::string message;
 static Warning *global_warning_ptr;
 Warning() {
 defaults();
 }
public:
 void defaults() {
 message = "no warnings";
 status = false;
 }
 void set(std::string v) {
 message = v;
 status = true;
 }
 void print() {
 if (status) {
 std::cout << message std::endl;
 defaults();
 }
 }
 static Warning *msg() {
 if (!global_warning_ptr)
 global_warning_ptr = new Warning;
 return global_warning_ptr;
 }
};
Warning *Warning::global_warning_ptr;

that is used by multiple functions as below

void bar(double x) {
 if (x < 0)
 Warning::msg()->set("Wrong values");
}
void foo(std::vector x) {
 int n = x.size();
 for (int i = 0; i < n; i++) {
 bar(x[i]);
 }
 Warning::msg()->print(); // print single warning if any error occures
}

However using singletons is often considered as a bad coding practice. Is there any better alternative or place for improvment in my code? Is there anything I should worry about when using it?

• BitTickler: In multithreaded your msg() function has a race condition. If you are out of memory (unlikely maybe), you could have fun effects, too. Also, in a chain of errors (error causes another error), you only see the (least interesting) last warning but not those leading to it. If you program embedded code without exceptions, I think bool Foo(double x) ... and error checks along the call path is superior. If you use exceptions, then simply throw and catch where convenient. Functions which "warn" about invalid arguments will most likely bail anyway.
• Cody Gray: Worth a read: C++ and the Perils of Double-Checked Locking
• Pieter Witvoet: This approach is very brittle. Calling foo twice, where only the first call fails, still causes both calls to print a warning, because you forgot to explicitly reset the global warning instance. If later it turns out that foo needs to call another method after calling bar, and that method also sets a warning, then any bar warning message is lost. Why don't you let bar return an error code, so foo can check it and log a warning for each bar call that fails (or a general warning if any call failed, depending on your needs)?

Answer 1

I don't want to sound harsh, but the code is useless. On top of that, the singleton seems to be slightly off in this situation. You should have a function which is a friend to the class and has static local variable and then simply return reference to it. The class itself should have private constructor, so that only the specified function can instantiate it.

Why?

• Because it is not thread safe
• Because it is singleton
• Because only one message is supported at a time
• No real way to retrieve the message as string (companies will need a person who looks at the monitor)
• Memory leak

Alternative:

CustomStream& debugStream()
{
 static CustomStream stream;
 return stream;
}

where CustomStream is thread safe stream possibly inhertied from std::ostream. Do note that static variables are initialized in a thread safe way only from C++11 and forward (there might be some compiler extensions that allow it prior to C++11).

• BitTickler: Much better. Only some coding guidelines (e.g. MISRA) disallow static member variables in functions. (testability and a few other reasons).
• Incomputable: @BitTickler, well, that's the only choice if you want a singleton. Also, you will get thread safe initialization for free since C++11.
• Tim: Actually bullets 3 & 4 are expected behavior: I want to record only if any error occurred and need to print them. I am looking how to improve the code as in my example while retaining it's behavior (record if any error occurred in if statement and print it in the end of main function).
• Incomputable: @BitTickler, sorry I thought you're the asker.
• Cody Gray: Judging by the volume of Turbo C questions I see appear on Stack Overflow, I'm not sure if that's true. :-) Aside from that, many compiler vendors were very slow to introduce support for C++11 features, and lots of shops cannot (or won't) for whatever reason upgrade their toolchains frequently, which means they're stuck with something that supports an indeterminate blend of C++03 and C++11. Thread-safe initialization of statics might not be among the supported features.