patterncppMinor
Thread-Safe Variadic Printing Function
Viewed 0 times
functionprintingvariadicthreadsafe
Problem
Printing to
stdout is thread-safe in many systems when using printf or std::cout, but not in all systems (Windows!). So I decided to make my own thread-safe and type-safe printing function with some help of C++11 variadic templates functions.#include
#include
#include
class Mutex:private std::mutex{
public:
class Lock:private std::lock_guard{
public:
Lock(Mutex& mutex):
std::lock_guard(mutex){}
};
};
class{
private:
Mutex m_mutex;
void Print(){}
template
void Print(const T& t,const Ts&... ts){
std::cout
void operator()(const T& t,const Ts&... ts){
Mutex::Lock lock(m_mutex);
Print(t,ts...);
}
}Print;
template
void PrintLine(){
Print('\n');
}
template
void PrintLine(const T& t,const Ts&... ts){
Print(t,ts...,'\n');
}
int main(){
Print(10," devided by ",3," equals ");
PrintLine(std::fixed,std::setprecision(5),10.0/3);
return 0;
}Solution
Mutex
Don't see the need for your custom Mutex/Lock class. Just use the standard ones directly there is no need to add a layer of indirection that future developers need to go and check.
Excessive Flushing
Calling flush after each item:
Is probably not a good idea. Let the user of your code decide when to flush he probably has more concept about what what is about to be printed and thus when flush needs to be called.
If you must do it then do it after all the items have been printed.
Template Recusrsion.
Rather than using a recursive call to print all the items:
Use a sub class and create a std::initializer_list
Then
As a nice benifit. You will not need the terminating case.
Naming Convention.
It is more standard to use an initial capitol letter to define a type. An initial lower case letter donates an object (which also encompasses functions).
Since you have a nameless type. With an object called
Declaration
The code works well as is for a single file program. But you have problems when using it from header files. Because the class is nameless you can not use it any declarations to mark the object external and thus you will be getting an instantiation of the object in every compilation unit. This will break your guarantee that it is thread safe as each has its own mutex.
I think your best bet is to put the
Expansion.
Currently your printer is only used for std::cout. Why not expand it so that it can be used for any stream.
class Mutex:private std::mutex{
public:Don't see the need for your custom Mutex/Lock class. Just use the standard ones directly there is no need to add a layer of indirection that future developers need to go and check.
Excessive Flushing
Calling flush after each item:
std::cout<<t<<std::flush;Is probably not a good idea. Let the user of your code decide when to flush he probably has more concept about what what is about to be printed and thus when flush needs to be called.
If you must do it then do it after all the items have been printed.
Template Recusrsion.
Rather than using a recursive call to print all the items:
Print(ts...);Use a sub class and create a std::initializer_list
// std::cout<<t<<std::flush;
// Print(ts...);
// Create a list. This will act more like a loop than recursion.
// Items inside the {} are initialized left to right.
auto printer = { ItemPrinter(ts)... };Then
ItemPrinter just prints the item in the constructor.template
struct ItemPrinter { ItemPrinter(T const& t){std::cout << t;}};As a nice benifit. You will not need the terminating case.
void Print(){}Naming Convention.
PrintIt is more standard to use an initial capitol letter to define a type. An initial lower case letter donates an object (which also encompasses functions).
Since you have a nameless type. With an object called
Print. I would have called it streamPrinter. Then used your wrapper functions call that directly.Declaration
The code works well as is for a single file program. But you have problems when using it from header files. Because the class is nameless you can not use it any declarations to mark the object external and thus you will be getting an instantiation of the object in every compilation unit. This will break your guarantee that it is thread safe as each has its own mutex.
I think your best bet is to put the
Print object in its own file. Then expose all printing via wrapper functions which can be made external in the header file.Expansion.
Currently your printer is only used for std::cout. Why not expand it so that it can be used for any stream.
Code Snippets
class Mutex:private std::mutex{
public:std::cout<<t<<std::flush;Print(ts...);// std::cout<<t<<std::flush;
// Print(ts...);
// Create a list. This will act more like a loop than recursion.
// Items inside the {} are initialized left to right.
auto printer = { ItemPrinter(ts)... };template<typename T>
struct ItemPrinter { ItemPrinter(T const& t){std::cout << t;}};Context
StackExchange Code Review Q#91622, answer score: 4
Revisions (0)
No revisions yet.