elements-of-python-style icon indicating copy to clipboard operation
elements-of-python-style copied to clipboard

To repeat or not to repeat?

Open dhirschfeld opened this issue 7 years ago • 2 comments

The current advice as given is:

# bad
class JSONWriter(object):
    handler = None
    def __init__(self, handler):
        self.handler = handler

# good
class JSONWriter(object):
    def __init__(self, handler):
        self.handler = handler

I actually prefer to define all the attributes on the class itself for several reasons:

  1. It defines the api in a single location at the top of the class - this is similar in principle to the advice to set all attributes in the constructor
  2. The attributes can be (briefly) documented with comments to help understanding the api
  3. You don't need to actually instantiate an instance of the class to use tab-completion to investigate the api. Sometimes for classes which take complicated object arguments, instantiating the class can be a non-trivial exercise (that may be a code smell itself, but it does happen)

This is the advice given in the ipython dev docs so perhaps the current advice could be supplemented with a note to the effect that there are different schools of thought on the matter

dhirschfeld avatar Jan 07 '17 03:01 dhirschfeld

@dhirschfeld You are probably right that there are multiple schools of thought on this issue. Thanks for the pointer to the IPython dev docs. I'll think through this a bit more and see if I can find other examples from popular open source projects.

amontalenti avatar Jan 07 '17 14:01 amontalenti

I kindly disagree. While there are some special cases (which should be listed), it doesn’t make sense to do this. I have these reasons:

  • You have to type the variable name twice, which is unpythonic.
  • Having the variable at class-level implies that the variable has class-level use. Having the class-level always be None is confusing and not right. If it should never be referenced at class-level, it should not be at the class-level, only the instance level.
  • As for your argument 1, because __init__ is usually at the top, it doesn’t seem pythonic to do it again.
  • As for your argument 2, don’t use comments, that isn’t right. Use a docstring instead.

However, I do see your point of argument 3, so it should probably be put in a side note.

Also, to the question title “To repeat or not to repeat?”, the answer is to not repeat. If one has to repeat anything in their code, that is a problem and can be changed to not have the repeatition.

Nc432o5 avatar Feb 06 '19 21:02 Nc432o5