On Thursday 03 November 2016 00:46, stest...@gmail.com wrote: > Hi > > I was hoping to canvas opinion on using classmethods as constructors over > __init__. > > We've got a colleague who is very keen that __init__ methods don't contain > any logic/implementation at all, and if there is any, then it should be moved > to a create() classmethod.
Is your colleague expecting the caller to directly call this create() method? In other words, is create() part of the public API for the users of the class, as in the following: instance = MyClass.create(arg) instead of this? instance = MyClass(arg) I think that's just being unconventional for the sake of being unconventional. The standard Python API for classes is to call the class name, we write: x = float(arg) d = dict(mapping, key=value) obj = object() rather than float.create(arg), dict.create(...) and object.create(). Go through the entire Python standard library, and you'll see that virtually ALL classes and types -- not just "primitives" like float and dict -- behave like this. There are a handful of classes which provide an Alternate Constructor API, e.g. dict.fromkeys(sequence_of_keys). But that offers a distinct, separate API for creating dicts. Hence the *alternate* part. The bottom line is, the standard way to call MyClass.create(arg) in Python is to leave out the ".create". But perhaps create is meant to be part of the *internal* implementation, not an external API. In that case, things get a bit more interesting... see below. > As a concrete example, one change proposed was to go from this: > > ``` > def __init__(self, ...): > self.queue = Queue.Queue() > > to this: > > def __init__(self, queue): > self.queue = queue That's the Dependency Injection design pattern. It's so trivially easy in Python that it hardly even deserves the name "Design Pattern". But trivial or not, it's very useful! You just have to write it the smart way. > @classmethod > def create(cls, ...): > return cls(Queue.Queue()) > This is not the smart way. This requires the caller to either create their own queue (occasionally useful, but not something you want to do all the time) or else call a non-standard constructor method. The smart way of doing optional dependency injection is like this: def __init__(self, queue=None): if queue is None: queue = Queue.Queue() self.queue = queue In more complex cases I can see the wisdom of writing a separate method: def __init__(self, widget=None): if widget is None: widget = self._build_widget(spam, eggs, cheese, tomato) self.widget = widget def _build_widget(self, a, b, c, d): ... return widget Note that the create method, _build_widget(), is flagged as private by the leading underscore. Users should not call it (or they do so at their own risk); subclasses may override or replace the _build_widget method. That may be what your colleague is thinking about. If so, then I can see that it *may* be a useful technique in some cases. But my guess is that he is over- engineering your classes and planning for things that will never happen. Remember KISS and YAGNI. You can always come back and insert a _build_widget() method into your class when you need it. > The rationale is that it decouples the class from the queue implementation > (no evidence or suggestion that we would actually ever want to change impl in > this case), and makes testing easier. > > I get the underlying principal, and it's one that a strict OOp approach would > suggest, but my gut feeling is that this is not a pythonic approach at all. Being able to pass in an optional dependency (the queue, in your example above) is a useful, Pythonic thing to do. Making it optional is even more Pythonic. But factoring out the call to Queue.Queue() into its own method just for the sake of OO purity is not very Pythonic, and *requiring* the caller to call this create() method is anti-Pythonic. -- Steven 299792.458 km/s — not just a good idea, it’s the law! -- https://mail.python.org/mailman/listinfo/python-list