5 ms·
Pretty simple to fix by changing the _init to something like: def _init(self): init_complete = threading.Event() def worker_thread_start():
by zokier 2y ago
Pretty simple to fix by changing the _init to something like:
def _init(self):
init_complete = threading.Event()
def worker_thread_start():
FooWidget.__init__(self)
init_complete.set()
self.run()
worker_thread = Thread(target=worker_thread_start, daemon=True)
worker_thread.start()
init_complete.wait()
Spawning worker thread from constructor is not that crazy, but you want to make sure the constructing is completed by the time you return from the constructor.
- ohr 2y agoThanks, I hate it! Jk, but would you consider this a good solution overall?
- gpderetta 2y agoI would consider this an hack. But a reasonable hack if you can't change the base class.
- im3w1l 2y agoNot him, but I would also consider making FooBarWidget have a FooWidget rather than be a FooWidget. Normally with a FooWidget you can create one on some thread and then perform operations on it on that thread. But in the case of the FooBarWidget you can not do operations on it because operations must be done in the special thread that is inaccessible.
- shiandow 2y agoI can't come up with a single good reason why you would wish to use inheritance in this case. Except possibly for type checking, but 1. Python has duck typing anyway 2. It's debatable whether these two classes should be interchangeable 3. You shouldn't need to use inheritance just to make the type checking work. It could be considered a clever hack in some situations, but it's completely unsurprising that it causes issues. Putting band-aids on it after you find a bug does not fix the real problem.
- greatgib 2y agoTo be clear, a good init is not supposed to create a thread or do any execution that might not be instant of things like that. It would have been better to an addition start, run, exec method that does that kind of things. Even if for usage it is an inch more complicated to use with an additional call.
- lmm 2y ago> It would have been better to an addition start, run, exec method that does that kind of things. Even if for usage it is an inch more complicated to use with an additional call. That breaks RAII - you shouldn't give the user a way to create the object in an invalid state. Although if it's intended to be used as a context manager anyway then maybe doing it on context enter rather than on init would be nicer.
- greatgib 2y agoDoesn't necessarily have to be in an invalid state. You can have your object with a none value for the socket or a state like "not_started" and the different method will look at that to handle everything properly. Also, it was not supposed to be used in a context manager as there is just a close method but it is the caller that decided to wrap it in a context. But as what you suggest, indeed it would have been a great idea to add an __enter__ and do the starting there are as it would be the proper way.
- crdrost 2y agoI don't think it's totally crazy to, say, open a DB connection in __init__() even though that's not an instantaneous operation. That's not a hill I would die on, you can just say “open a connection and hand it to me as an argument,” but it just looks a little cleaner to me if the lifecycle of the connection is being handled inside this DB-operations class. (You can also defer creating the connection until it is actually used, or require an explicit call to connect it, but then you are also writing a bunch of boilerplate to make the type checker happy for cases where the class is having its methods called before it was properly connected.)
- 2y ago