4 ms·

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 compl

by mjr00 2y ago

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.

abdusco 2y ago
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)
echelon 2y ago
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.
- fouronnes3 2y ago
  How would you do traits in Python?
  - echelon 2y ago
    I'd really need to think long and hard about it, but my initial feeling is that we'd attach them to data classes or a similar new construct. I don't think you'd want to reason about the blast radius with ordinary classes. Granted, that's more language complexity, creates two unequal systems, and makes much more to reason about. There's a lot to juggle. As much fun as putting a PEP together might be, I don't think I have the bandwidth to do so. I would really like to see traits in Python, though.
  - svilen_dobrev 2y ago
    there were "traits" in python :/ . Search for pyprotocols. (searching myself.. not much left of it) 2004 - https://simonwillison.net/2004/Mar/23/pyprotocols/ https://simonwillison.net/2004/Mar/23/pyprotocols/ some related rejected PEP .. https://peps.python.org/pep-0246/ https://peps.python.org/pep-0246/ talking about something new to be expected.. in 2001?2005? no idea did it happen and which one would that be..
    - dragonwriter 2y ago
      Possibly Guido, in that rejection, was talking about what ended up as ABCs, original PEP: https://peps.python.org/pep-3119/ https://peps.python.org/pep-3119/ Further work was done in this area, building on ABCs, with Protocols, original PEP: https://peps.python.org/pep-0544/ https://peps.python.org/pep-0544/
  - vaylian 2y ago
    Python has a trait-like system. It's called "abstract base classes": https://docs.python.org/3/library/abc.html https://docs.python.org/3/library/abc.html The abstract base class should use the @abstractmethod decorator for methods that the implementing class needs to implement itself. Obviously, you can also use abstract base classes in other ways, but they can be used as a way to define traits/interfaces for classes.
rcxdude 2y ago
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.
- deleted 2y ago
  [deleted]
- mjr00 2y ago
  It's a lot easier to reason about code if I don't need to worry that something as simple as my_variable = MyType() might be calling out to a database with invalid credentials, establishing a network connection which may fail to connect, or reading a file from the filesystem which might not exist. You are correct that you don't want an object that can be half-initialized. In that case, any external resources necessary should be allocated before the constructor is called. This is particularly true if the external resources must be closed after use. You can see my other comment[0] in this thread for a full example, but the short answer is use context managers; this is the Pythonic way to do RAII. [0] https://news.ycombinator.com/item?id=43736940 https://news.ycombinator.com/item?id=43736940
  - rcxdude 2y ago
    Why, when my_var = MyType() my_var.actually_init() Works in exactly the same way? Errors get reported the same, it has the same semantics, you have the same need to keep track of things, it's just two lines instead of one and you're more likely to have a partially initialised object floating around. I really don't see the advantage at all. Context managers are handy if you need to guarantee cleanup in simple object lifetimes, and in that case __enter__ is probably the better place to do setup because it's closer to where you get guarantees about the cleanup happening, but context managers are not actually RAII, just a partial substitute.
    - 2y ago
jhji7i77l 2y ago
> 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).
- mjr00 2y ago
  You're right that consistent internal state is important, but you can accomplish this with class MyClass: def __init__(self, config: str): self._config = config And if your reaction is "that just means something else needs to call Path("config.json").read_text()", you're absolutely right! It's separation of concerns; let some other method deal with the possibility that `config.json` isn't accessible. In the real world, you'd presumably want even more specific checks that specific config values were present in the JSON file, and your constructor would look more like def __init__(self, host: str, port: int): and you'd validate that those values are present in the same place that loads the JSON. This simple change makes code so much more readable and testable.
  - asplake 2y ago
    Better still, pass the parsed config. Let the app decide how it is configured, and let it deal with any problems.

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 compl

by mjr00 2y ago

abdusco 2y ago

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)

echelon 2y ago

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.

fouronnes3 2y ago
How would you do traits in Python?
- echelon 2y ago
  I'd really need to think long and hard about it, but my initial feeling is that we'd attach them to data classes or a similar new construct. I don't think you'd want to reason about the blast radius with ordinary classes. Granted, that's more language complexity, creates two unequal systems, and makes much more to reason about. There's a lot to juggle. As much fun as putting a PEP together might be, I don't think I have the bandwidth to do so. I would really like to see traits in Python, though.
- svilen_dobrev 2y ago
  there were "traits" in python :/ . Search for pyprotocols. (searching myself.. not much left of it) 2004 - https://simonwillison.net/2004/Mar/23/pyprotocols/ https://simonwillison.net/2004/Mar/23/pyprotocols/ some related rejected PEP .. https://peps.python.org/pep-0246/ https://peps.python.org/pep-0246/ talking about something new to be expected.. in 2001?2005? no idea did it happen and which one would that be..
  - dragonwriter 2y ago
    Possibly Guido, in that rejection, was talking about what ended up as ABCs, original PEP: https://peps.python.org/pep-3119/ https://peps.python.org/pep-3119/ Further work was done in this area, building on ABCs, with Protocols, original PEP: https://peps.python.org/pep-0544/ https://peps.python.org/pep-0544/
- vaylian 2y ago
  Python has a trait-like system. It's called "abstract base classes": https://docs.python.org/3/library/abc.html https://docs.python.org/3/library/abc.html The abstract base class should use the @abstractmethod decorator for methods that the implementing class needs to implement itself. Obviously, you can also use abstract base classes in other ways, but they can be used as a way to define traits/interfaces for classes.

rcxdude 2y ago

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.

deleted 2y ago
[deleted]
mjr00 2y ago
It's a lot easier to reason about code if I don't need to worry that something as simple as my_variable = MyType() might be calling out to a database with invalid credentials, establishing a network connection which may fail to connect, or reading a file from the filesystem which might not exist. You are correct that you don't want an object that can be half-initialized. In that case, any external resources necessary should be allocated before the constructor is called. This is particularly true if the external resources must be closed after use. You can see my other comment[0] in this thread for a full example, but the short answer is use context managers; this is the Pythonic way to do RAII. [0] https://news.ycombinator.com/item?id=43736940 https://news.ycombinator.com/item?id=43736940
- rcxdude 2y ago
  Why, when my_var = MyType() my_var.actually_init() Works in exactly the same way? Errors get reported the same, it has the same semantics, you have the same need to keep track of things, it's just two lines instead of one and you're more likely to have a partially initialised object floating around. I really don't see the advantage at all. Context managers are handy if you need to guarantee cleanup in simple object lifetimes, and in that case __enter__ is probably the better place to do setup because it's closer to where you get guarantees about the cleanup happening, but context managers are not actually RAII, just a partial substitute.
  - 2y ago

jhji7i77l 2y ago

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

mjr00 2y ago
You're right that consistent internal state is important, but you can accomplish this with class MyClass: def __init__(self, config: str): self._config = config And if your reaction is "that just means something else needs to call Path("config.json").read_text()", you're absolutely right! It's separation of concerns; let some other method deal with the possibility that `config.json` isn't accessible. In the real world, you'd presumably want even more specific checks that specific config values were present in the JSON file, and your constructor would look more like def __init__(self, host: str, port: int): and you'd validate that those values are present in the same place that loads the JSON. This simple change makes code so much more readable and testable.
- asplake 2y ago
  Better still, pass the parsed config. Let the app decide how it is configured, and let it deal with any problems.