-
I just wanted to get an old (and never finished) project to work with SFML 2.0. I found out that there were lots of changes to the sf::Thread class, and I'm a bit confused why the 'old' way of creating threads (by inherting from sf:Thread and overriding 'Run()') was removed - I found this approach simple and elegant.
What's the reason for this change? Are there plans to reimplement the 'old' approach as an alternative?
-
The new version is much cleaner and more flexible. The old way is just a very limited subset of the new API:
sf::Thread thread(this, &MyClass::Run);
-
I do not doubt that the new version is more flexible and powerful, but I'm not sure whether it's really cleaner.
If you have a class where each object should be 'active' (i.e., a separate thread), the old approach was well-suited and clean. I guess you could put the code you posted in the constructor of such a class to achieve that there's a thread per object without having to deal with Thread objects outside that specific class, but the resulting code doesn't seem to be cleaner or simpler.
btw: Shouldn't the parameters in your example be the other way round? I don't find a matching constructor in the documentation.
-
the resulting code doesn't seem to be cleaner or simpler
It's cleaner because you don't inherit the public API of sf::Thread. It becomes a pure implementation detail. Well, you could use private inheritance but I think most people used public inheritance by habit.
btw: Shouldn't the parameters in your example be the other way round? I don't find a matching constructor in the documentation.
Oops, you're right. Sorry.
-
Thank you for your answer.
It's cleaner because you don't inherit the public API of sf::Thread. It becomes a pure implementation detail. Well, you could use private inheritance but I think most people used public inheritance by habit.
I know too little about C++ inheritance - why is it a problem if a class inherits the public API from sf::Thread? Being able to call something like myObject.Wait(); seems quite natural if the object runs within a separate thread.
Oops, you're right. Sorry.
No problem. I just was not sure if I correctly understood the API at that point. I'm not very good with that template stuff - probably that's the reason I liked the old API better ;)
-
I know too little about C++ inheritance - why is it a problem if a class inherits the public API from sf::Thread?
It's always better to decide which functions you want, with your own names and naming convention. Instead of being forced to adopt all the functions of sf::Thread (you most likely don't want terminate() in your own class), with the names that I chose (maybe you prefer join() to wait()), and the naming convention of SFML (maybe you prefer Launch() instead of launch()), etc.
It's not hard to forward functions of an internal member in your own class, but it's impossible to hide or rename functions inherited from a public base.
-
I know too little about C++ inheritance - why is it a problem if a class inherits the public API from sf::Thread?
Additional to what Laurent said: The class has to know SFML and to depend on sf::Thread. In contrast, the current approach works with any callable type (function, functor, lambda expression), especially with already existing ones that aren't adapted to SFML.
-
Yes, the new approach surely is more flexible, and I don't propose to replace it with the old one. I was just wondering why the old way wasn't kept as an alternative (especially as the old API also offered a similar constructor, taking a function pointer).
-
It is implicitely included in the new API, as I showed you above
sf::Thread thread(&MyClass::Run, this);
(in case you didn't notice, thread can be a private member of MyClass and be initialized in the constructor -- so from the outside point of view there's no difference).
There's really no need to rewrite the inheritance stuff in addition to that.
-
It is implicitely included in the new API, as I showed you above
sf::Thread thread(&MyClass::Run, this);
(in case you didn't notice, thread can be a private member of MyClass and be initialized in the constructor -- so from the outside point of view there's no difference).
Yes, I did understand that - it's more the "nicer and cleaner" argument from above (and perhaps backward-compatibility) that made me miss the old API a little bit. ;)
-
thread can be a private member of MyClass and be initialized in the constructor
How would you do that? Because I can't go like this: //in the class header
sf::Thread thread;
//...
// in the constructor
thread = sf::Thread(&MyClass::Run, this);
because thread is NonCopyable.
-
Use the constructor initializer list. You should always prefer it over assignments in the constructor body.
-
Ah alright. Thank you very much!
-
Use the constructor initializer list. You should always prefer it over assignments in the constructor body.
When I do that in Visual C++ 2010 it gives me a warning:warning C4355: 'this' : used in base member initializer list
Is there a good way to avoid that, or should I just not mind? Simply suppressing the warning is not a good solution in my opinion.
-
I am sad it still can't run on FreeBSD.
-
warning C4355: 'this' : used in base member initializer list
Using 'this' in the initializer list triggers a warning on VC++, yes. The only way to get rid of it is to disable it. Or to allocate your sf::Thread instance dynamically so that it can be constructed outside the initializer list.
I am sad it still can't run on FreeBSD.
If you have something relevant to say related to this thread, please say it clearly.
-
Is there a good way to avoid that, or should I just not mind? Simply suppressing the warning is not a good solution in my opinion.
You can disable it locally with #pragma warning(push)
#pragma warning(disable: 4355)
// code
#pragma warning(pop)
I wouldn't use dynamic allocations just to avoid the warning.
-
It's generally not a good idea to pass this in the initializer list because at that point it's not finished being constructed. However, in this case you know that Thread isn't going to access it until you later call .launch(), so it's safe to ignore the warning.
I don't think there's a way to avoid the warning given the current API.
-
I don't think there's a way to avoid the warning given the current API.
There will be one when C++11 features are added: the move assignment operator.
-
Would that really be able to avoid the warning? I thought Thread didn't have a default constructor.
-
I think it would have one in this case.
-
I think it would have one in this case.
In case you mean "default constructor" with "one": Do you really want to allow invalid (default-constructed) threads to avoid this warning?
-
In case you mean "default constructor" with "one": Do you really want to allow invalid (default-constructed) threads to avoid this warning?
Not just to avoid this warning, but to allow defining a thread after its construction, without forcing dynamic allocation.
-
And what happens when launch(), wait() or terminate() is called on an invalid thread? I don't know if just doing nothing is a good idea...
-
Launch: This is the one stickler. The stl gets around this because threads to launch as soon as constructed, instead of a manual launch function. I guess you could just make launch() throw or return some kind of error?
Wait: Would wait not just do nothing? It's a join command, a finished thread just keeps going here (well, with std at least throwing on any exceptions in the process), so I'd assume an invalid thread does nothing too...
Terminate: Is there a context in which SFML will be used where any way of forcibly terminating a thread is the best option? That aside, again how is are invalid thread and a finished thread different to an outside observer? Terminating a finished thread, presumably does nothing.
In the interests of honest, I've never used SFML's threading library. Always favoured boost threads, or C++11 threads when available (To simplify this I actually wrote a bit of code to drag in and turn a boost thread into the appropriate std:: versions with correct functions and parameters).
-
If a default-constructed thread is considered "empty" (no function), it makes sense to do nothing in these functions.
Anyway, the problem is still: how to allow changing the definition of a thread after its construction, without dynamic allocation. It's not about allowing or not a default constructor ;)
-
Well without move constructors or similar (how boost fakes it), the only solution I can think of (and I'm sure you have already considered) is a "construct" or "setFunction" function or something? Which isn't the prettiest solution...
Then again, functionally I guess that isn't much different from a move constructor.
-
In case you mean "default constructor" with "one": Do you really want to allow invalid (default-constructed) threads to avoid this warning?
Not just to avoid this warning, but to allow defining a thread after its construction, without forcing dynamic allocation.
With enough Boost, this is already kind of possible (http://www.boost.org/doc/libs/1_49_0/libs/optional/doc/html/index.html).
-
With enough Boost, this is already kind of possible.
That works for people who use boost, but it would be cool to have a solution that works for others too, something built in SFML.
-
That works for people who use boost, but it would be cool to have a solution that works for others too, something built in SFML.
Sure, I'm just saying that if someone badly needs that right now, it's possible.