4 ms·
How to improve this tutorial: - Use lambdas - Use std::scoped_ptr or std::shared_ptr - Use std::vector instead of C Array - Use references instead of pointe
by shin_lao 15y ago
How to improve this tutorial:
- Use lambdas
- Use std::scoped_ptr or std::shared_ptr
- Use std::vector instead of C Array
- Use references instead of pointers
That would prevent unnecessary pitfalls.
- exDM69 15y agoAgreed. But, see this comment in the source: //It may seem strange to not use a vector here, however (at the time of this writing) clang++ doesn't let you use a vector to store a group of threads AFAIK, it should be possible to put threads (non-copyable but movable) to a vector in C++11. Of course, then the vector will not be copyable (but it will be movable). I think I've even done it, but I used GCC, which has had pretty good C++11 support for quite a while now. (sorry, I'm too lazy to verify)
- octopus 15y agoSame goes for lambdas, clang++ has no support for lambdas.
- shin_lao 15y agoI'm not sure it works even if it's moveable because there's no guarantee it will make copy elision everywhere it's needed. One way I was thinking about it was std::vector<std::shared_ptr<std::thread>> Additionally, this sucks: #define NUM_THREADS 10 In C++03 you can write static const int num_threads = 10; Which is much better. And instead of: for(int i=0;i<NUM_THREADS;i++){ t[i] = thread(call_from_thread,i); } Better use std::generate. I may look like I'm nitpicking but I think it's paramount in a tutorial to have all details really, really right so that it may trigger other questions and go in depth about why things are better a way or the other.
- exDM69 15y ago>> I'm not sure it works even if it's moveable because there's no guarantee it will make copy elision everywhere it's needed. Of course it works, since std::thread is movable but non-copyable, std::vector<std::thread> will be movable but non-copyable too. So the compiler type checking will make sure it's not copied anywhere. This works like a charm (with GCC 4.7-ish from git): #include <thread> #include <vector> #include <iostream> #include <algorithm> int main() { std::vector<std::thread> threads; for(int i = 0 ; i < 10 ; ++i) threads.push_back(std::thread([=]() { std::cout << i << std::endl; })); std::for_each(threads.begin(), threads.end(), [](std::thread& thread) { thread.join(); }); } >> Better use std::generate. I used to nitpick about not using std::algorithms when they're available, but I grew out of it. I figured out that std::algos are very limited (esp. in c++98), and while you can use them to do stuff, it usually takes a significant amount of brainpower only to reformulate your problem to be suitable for std::algorithms. There are so many situations where a good old loop will produce more readable code faster, so why bother with the standard algorithms if they're not a perfect fit? Btw. has std::generate improved at all in C++11? It's one of the most limited algorithms in c++98.
- shin_lao 15y ago>> it usually takes a significant amount of brainpower only to reformulate your problem to be suitable for std::algorithms That's true, but the final result generally ends up with less bugs and better thought out algorithms. It forces you to think "what problem am I currently trying to solve?"
- steve-howard 15y agoI don't have much experience with clang, but there are still better ways to express a list of threads: - thread t[NUM_THREADS]; // since NUM_THREADS is constant - std::unique_ptr<thread[]> t(new thread[NUM_THREADS])
- thelema314 15y ago> - Use std::vector instead of C Array What about std::array?
- octopus 15y agoIt will work with std::array.