Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

To repeat or not to repeat? #34

Open
dhirschfeld opened this issue Jan 7, 2017 · 2 comments
Open

To repeat or not to repeat? #34

dhirschfeld opened this issue Jan 7, 2017 · 2 comments

Comments

@dhirschfeld
Copy link

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

@amontalenti
Copy link
Owner

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

@Nc432o5
Copy link

Nc432o5 commented Feb 6, 2019

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants