Best practices python classes [closed]

Question

Closed. This question is opinion-based. It is not currently accepting answers.

---

Want to improve this question? Because this question may lead to opinionated discussion, debate, and answers, it has been closed. You may edit the question if you feel you can improve it so that it requires answers that include facts and citations or a detailed explanation of the proposed solution. If edited, the question will be reviewed and might be reopened.

Closed 2 years ago.

Improve this question

I have some code that looks roughly like

class Foo:
 def __init__(self, a, b, c=None):
 self.bar = a
 if c:
 self.data = self.calc(b, c)
 def calc(self, bat, baz):
 """calculates some stuff that feels like should belong with the instance of `Foo`"""

1. Is it bad to have code within my __init__?

2. If I have a method that generates some data relating to the class instance, is it better practice to leave the user to call that method and store the data, or have one of the instance variables store it upon instantiation?

The reason for 2 is because I have this class where everything can be generated from some json file, so should the class just have one instance variable and a bunch of methods? Should this not even be a class? I always thought that classes with one instance variable were bad practice, but on the other hand, I have a group of functions that all operate on the same json file so they feel like they should belong in some conglomerate structure.

EDIT: As a third question, relating to 2, what if some of the methods require data that depends on the initialized data, and calculated data. So in the example above, suppose I (the user) calculate d=2*a+b because it is of interest to me. Then I want to do another computation that in theory would be sped up if I gave it d. So a method that looked like new_calc(self, d). Should I store d in the class when I calculate it initially, then not ask the user for it, or should I make them more responsible?

• bruno desthuilliers: Just one point: you should set self.data to a sensible default value if c is None. Also note that bool(c) might eval to False for (at least) empty lists, empty tuples, empty sets, empty dicts, empty strings and numeric zeros (ints, floats or Decimal).
• Nate Stemen: @brunodesthuilliers right of course. i may have made a MWE that isn't very good practice... oops
• bruno desthuilliers: Also if you're using Python 2.x, you want to inherit from object to get a "new-style" class that will properly support computed attributes and super() calls.
• JL Peyret: I remember reading somewhere that good OO design aims to minimize the need for method sequencing/dependency by outside code - i.e. remember to call f() before you call g(). Depending on compute cost and likelyhood d will end up being needed, sure why not @ init. On the other hand, a property w cache might work too.

Answer 1

Is it bad to have code within my init?

No. But if it does anything complicated, factor out the code into a function/method, just like you did with calc. Only I'd suggest that you use _calc instead of calc if the only caller of that method is the constructor. That way, you signal to users of the class that they're not meant to call _calc themselves.

A nitpick:

if c:
 self.data = self.calc(b, c)

I'd be wary of this code, because if c is None (or 0, or False), self.data remains unset. So all the rest of your methods have to check whether the data attribute exists. It might be better to write

self.data = self.calc(b, c) if c is None else None

Or you could just move the check for None into calc method and have the line in the constructor say

self.data = self.calc(b, c)

is it better practice to leave the user to call that method

I'd think that depended on the exact use case. If each instance represented a specific data set, and there was no need to swap out one set of data for another one, I'd say it makes more sense for the constructor to do it. This way, the data is nicely encapsulated by the instance, and the user can't make a stupid mistake by changing the data when it isn't supposed to change.

• Nate Stemen: right, make calc private, and yes as @brunodesthuilliers pointed out self.dat should be set to something regardless. thanks for your answer!