1

I'm using PySide2 to define my tool's interface, and I generally initialize all interface items outside __init__ as to not bloat it (any other important variables stay in __init__).

Unfortunately for me, I'm using PyCharm as my editor and it's giving me tons of warnings:

Instance attribute 'foobar' defined outside __init __

Here's a simple example of what I would be doing:

from PySide2 import QtWidgets


class MyTool(QtWidgets.QWidget):

    def __init__(self, parent=None):
        super(MyTool, self).__init__(parent)

        self.create_gui()

    def create_gui(self):
        # Complains about all variables below!

        self.awesome_checkbox = QtWidgets.QCheckBox(parent=self)

        self.awesome_button = QtWidgets.QPushButton(parent=self)

        self.awesome_label = QtWidgets.QLabel(parent=self)

        self.main_layout = QtWidgets.QVBoxLayout()
        self.main_layout.addWidget(self.awesome_checkbox)
        self.main_layout.addWidget(self.awesome_button)
        self.main_layout.addWidget(self.awesome_label)
        self.setLayout(self.main_layout)

Now I know one solution would be to initialize these variables in __init__ as None, but I can have fairly complex interfaces so it would be very long winded.

My question is if what I'm currently doing truly blasphemy? I know the variables are technically outside __init__, but the method is being called in the constructor anyways!

WhatsThePoint
  • 3,395
  • 8
  • 31
  • 53
Green Cell
  • 4,677
  • 2
  • 18
  • 49
  • @eyllanesc I disagree with this being a duplicate. That question is talking about any variable being outside the constructor, which I already think is wrong. I'm talking about variables specific for generating an interface, as I find it completely bloats `__init__`. That "duplicate" question doesn't answer my question. – Green Cell Dec 14 '18 at 02:03
  • The question is general: is it correct to define variables outside the constructor? It does not matter if it is a GUI or not. – eyllanesc Dec 14 '18 at 02:05
  • It still doesn't answer my question. The accepted answer says it's for clarity, but I think it does the exact opposite. Oh well, I'm damned if I do, and damned if I don't :) – Green Cell Dec 14 '18 at 02:09
  • Exactly, every rule can be broken and does not imply that it is bad or good. Even PEPs are not mandatory, they are recommendations to have a better code, but not using them does not imply obtaining a bad code. So it depends on the developer. In conclusion it is neither good nor bad, it may in some cases cause problems but not in your current example as it is consistent. :-) – eyllanesc Dec 14 '18 at 02:12

1 Answers1

0

Well, the short answer is: no, it's not "truly blasphemy".

It's considered good practice to create all the instance attribute and ensure they are in a consistant state in the initializer because it makes code easier to read (you only have one method to read to know what attributes your object has) and avoids potential AttributeError when an attribute is created by a method that might not always been called before the attribute is accessed. That's why most linters will (by default) warn you about this, and by itself it's a good thing as it can help you spot a potential bug before it makes it's way in production.

Now there are indeed cases where it makes sense to delegate part of the instance initialisation to a distinct method, ie when those attributes depends on each other and some other external factor and might have to be reset / updated together during the instance's lifecycle, or when you want to let child classes override this part of the initialization without having to override the __init__() method itself (cf the GOF's "template method" pattern).

In the case of a class with complex initialization (and this is typical of GUI components) it can also make sense to split the setup in distinct methods for readability reasons - a 50+ lines initializer is not really optimal when it comes to readability - so as far as I'm concerned I would probably do something similar with possibly a couple improvements: first make this a "protected" method (naming it _create_gui() - the leading underscore being the naming convention for protected attributes / methods) and then adding a guard to prevent the method from being executed twice (assuming this method is only supposed to be called once from the initializer and is not supposed to be part of the public API, of course). And then I would add a couple linter directives (those are specially formatted comments that the linter looks for) to make clear for both the linter and anyone reading this code that doing so was a deliberate design choice and not a rookie mistake.

bruno desthuilliers
  • 75,974
  • 6
  • 88
  • 118