Frankenstein's `__init__`

[mjr00]

Doing anything like this in __init__ is crazy. Even `Path("config.json").read_text()` in a constructor isn't a good idea.

Friends don't let friends build complicated constructors that can fail; this is a huge violation of the Principle of Least Astonishment. If you require external resources like a zeromq socket, use connect/open/close/etc methods (and a contextmanager, probably). If you need configuration, create a separate function that parses the configuration, then returns the object.

I appreciate the author's circumstances may have not allowed them to make these changes, but it'd drive me nuts leaving this as-is.

[echelon]

Not just Python. Most languages with constructors behave badly if setup fails: C++ (especially), Java, JavaScript. Complicated constructors are a nuisance and a danger.

Rust is the language I'm familiar with that does this exceptionally well (although I'm sure there are others). It's strictly because there are no constructors. Constructors are not special language constructs, and any method can function in that way. So you pay attention to the function signature just like everywhere else: return Result<Self, T> explicitly, heed async/await, etc. A constructor is no different than a static helper method in typical other languages.

new Thing() with fragility is vastly inferior to new_thing() or Thing::static_method_constructor() without the submarine defects.

Enforced tree-style inheritance is also weird after experiencing a traits-based OO system where types don't have to fit onto a tree. You're free to pick behaviors at will. Multi-inheritance was a hack that wanted desperately to deliver what traits do, but it just made things more complicated and tightly coupled. I think that's what people hate most about "OOP", not the fact that data and methods are coupled. It's the weird shoehorning onto this inexplicable hierarchy requirement.

I hope more languages in the future abandon constructors and strict tree-style and/or multi-inheritance. It's something existing languages could bolt on as well. Loose coupling with the same behavior as ordinary functions is so much easier to reason about. These things are so dated now and are best left in the 60's and 70's from whence they came.

[rcxdude]

Why? An object encapsulates some state. If it doesn't do anything unless ypu call some other method on it first, it should just happen in the constructor. Otherwise you've got one object that's actually two types: the object half-initialised and the object fully initialised, and it's very easy to confuse the two. Especially in python there's basically no situation where you're going to need that half-state for some language restriction.

[abdusco]

My go-to is a factory classmethod:

    class Foo:
        def __init__(self, config: Config): ...

        @classmethod
        def from_config_file(cls, filename: str):
          config = # parse config file
          return cls(config)

[jhji7i77l]

> Even `Path("config.json").read_text()` in a constructor isn't a good idea.

If that call is necessary to ensure that the instance has a good/known internal state, I absolutely think it belongs in __init__ (whether called directly or indirectly via a method).

[zokier]

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.

[greatgib]

To 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.

[ohr]

Thanks, I hate it! Jk, but would you consider this a good solution overall?

[wodenokoto]

> solving an annoying problem with a complete and utter disregard to sanity, common sense, and the feelings of other human beings.

While I enjoy comments like these (and the article overall!), they stand stronger if followed by an example of a solution that regards sanity, common sense and the feelings of others.

[crdrost]

So in this case that would be, a FooBarWidget is not a subclass of FooWidget but maybe still a subclass of AbstractWidget above that. It contains a thread and config as its state variables. That thread instantiates a FooWidget with the saved config, and runs it, and finally closes its queue.

The problem still occurs, because you have to define what it means to close a FooBarWidget and I don't think python Thread has a “throw an exception into this thread” method. Just setting the should_exit property, has the same problem as the post! The thread might still be initing the object and any attempt to tweak across threads could sometimes tweak before init is complete because init does I/O. But once you are there, this is just a tweak of the FooWidget code. FooWidget could respond to a lock, a semaphore, a queue of requests, any number of things, to be told to shut down.

In fact, Python has a nice built-in module called asyncio, which implements tasks, and tasks can be canceled and other such things like that, probably you just wanted to move the foowidget code into a task. (See @jackdied's great talk “Stop Writing Classes” for more on this general approach. It's not about asyncio specifically, rather it's just about how the moment we start throwing classes into our code, we start to think about things that are not just solving the problem in front of us, and solving the problem in front of us could be done with just simple structures from the standard library.)

[pjot]

Rather than juggling your parent’s __init__ on another thread, it’s usually clearer to:

1. Keep all of your object–initialization in the main thread (i.e. call super().__init__() synchronously).

2. Defer any ZMQ socket creation that you actually use in the background thread into the thread itself.

[ledauphin]

yeah, this just feels like one of those things that's begging for a lazy (on first use) initialization. if you can't share or transfer the socket between threads in the first place, then your code will definitionally not be planning to use the object in the main thread.

[smitty1e]

To paraphrase Wheeler/Lampson: "All problems in python can be solved by another level of indiscretion."

[jerf]

Oh, that's good. That has some depth to it. I'm going to have to remember that one. Of course one can switch out "Python" for whatever the local situation is too.

[_Algernon_]

fix: add time.sleep(0.1) in the .close method.

[gnfargbl]

    # NOTE: doubled this to 0.2s because of some weird test failures.

[Const-me]

Using sleep instead of proper thread synchronization is unreliable. Some people have slow computers like a cheap tablet or free tier cloud VM. Other people have insufficient memory for the software they run and their OS swaps.

When I need something like that, in my destructor I usually ask worker thread to shut down voluntarily (using a special posted message, manual reset event, or an atomic flag), then wait for the worker thread to quit within a reasonable timeout. If it didn’t quit within the timeout, I log a warning message if I can.

[devrandoom]

That's Python's constructor, innit?