Refactor tools/

[mottosso]

Goal

Reduce duplication, simplify implementations of tools.

Motivation

Our "tools" are GUIs written in Qt, and GUIs are a hard problem to solve. There is no PEP 008 for writing graphical applications, and the result is a lack of consistency and foresight.

• There's architecture - "MVC", do we need it, how do we apply it effectively?
• There's structure - pages, panels, widgets and layouts; how do we structure those?
• There's reactivity - which signals should exist and how do we handle them?
• There's communication - which widgets should talk to which, and which should go through a controller?
• There's controllers - What are they, what are their responsibilities?
• There's responsiveness - Anything made asynchronous multiplies every problem by 10x
• There's division of labour - do we create 3 small GUIs that do one thing each well, or 1 GUI that does everything?

Without appropriate answers to these questions, extending or editing a graphical application is a problem made even harder.

Implementation

Let's see if we can run through these and establish a foundation upon which to build GUIs effectively. I'll write these as unambious as I can, but remember these are guidelines and though some are tried and true, some are not (I'll tag these appropriately).

• Architecture
• Structure
  • View
  • Controller
  • Model
• Responsiveness

Architecture

You generally won't need to divide an application into model, view and controller unless it is of a certain size. But as this is an article on consistency, let's move the goal posts to say "No application is small enough to not require MVC".

With MVC as our baseline, here's how to implement it.

Module	Description
view.py	The paintbrush. The "main" of our Window, the cortex of where all widgets come together. This is responsible for drawing to the screen and for converting user interaction to interactions with the "controller"
control.py	The brain. The "main" of our program, independent of a display, independent of the data that it operates on.
model.py	The memory. The container of data.

These three are all that is required to implement an application. Additional modules are for convenience and organisation only, such as..

Module	Description
widgets.py	Independent widgets too large to fit in view.py, yet specific to an application (i.e. not shared with anything else)
util.py	Like widgets, but non-graphical. Standalone timers, text processing, threading or delay functions.
delegates.py	If relevant; these create a dependency between model and view, which typically only communicate through the controller.

Open Questions

• [ ] Q1 The model deals with data, the controller with logic. It's unclear whether the controller or model should interact with the database or perform complex data transformation. On the one hand, it's consistent having all logic - including access to the outside world - happen in the controller alone, leaving the model a mere data repository. Dumb data. But there are times when logic fits better with the model, such as caching of data. Sometimes, the data retrieval is unimportant to the overall function of the program and distracts from what the controller is doing, in which case the model should act as a thin API towards the controller.

Structure I - View

This involves the def __init__() in view.py.

class Window(QtWidgets.QMainWindow):
    title = "My Window"

    def __init__(self, ctrl, parent=None):
        super(Window, self).__init__(parent)
        self.setWindowTitle(self.title)
        self.setWindowIcon(QtGui.QIcon(...))
        self.setAttribute(QtCore.Qt.WA_StyledBackground)

        pages = {
            "home": QtWidgets.QWidget(),
        }

        panels = {
            "header": QtWidgets.QWidget(),
            "body": QtWidgets.QWidget(),
            "footer": QtWidgets.QWidget(),
            "sidebar": QtWidgets.QWidget(),
        }

        widgets = {
            "pages": QtWidgets.QStackedWidget(),
            "logo": QtWidgets.QWidget(),
            "okBtn": QtWidgets.QPushButton("Ok"),
            "cancelBtn": QtWidgets.QPushButton("Cancel"),
            "resetBtn": QtWidgets.QPushButton("Reset"),
        }

        icons = {
            "cancelBtn": resources.icon("cancelBtn"),
        }

Notes

• S1 Widgets MUST are separated into pages, panels and widgets
  • Pages are the full screen of an application, except statusbar, docks, toolbar and menu. Most apps only have one page, but some - like Pyblish Lite and QML - have more
  • Panels are subdivisions of a Page
  • Widgets are self explanatory
• S2 Widgets MUST be declared at the top
• S3 Widgets MUST have a mixedCase name, used in CSS
• S4 Window MUST have title declared as class attribute, used to dynamically change window title at run-time, e.g. to reflect a selection or state
• S5 Like widgets, icons SHOULD be declared up-front
• S6 Resource logic and I/O SHOULD be handled separately, for testing

class Window(QtWidgets.QMainWindow):
    title = "My Window"

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

        ...

        for name, widget in chain(panels.items(),
                                  widgets.items(),
                                  pages.items()):
            # Expose to CSS
            widget.setObjectName(name)

            # Support for CSS
            widget.setAttribute(QtCore.Qt.WA_StyledBackground)

        self.setCentralWidget(widgets["pages"])
        widgets["pages"].addWidget(pages["home"])

Notes

• S10 Every widget MUST be given a unique name
• S11 Every widget MUST be stylable with CSS
• S12 Central widget MUST be a QStackedWidget to facilitate multiple pages

class Window(QtWidgets.QMainWindow):
    title = "My Window"

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

        ...

        layout = QtWidgets.QHBoxLayout(panels["header"])
        layout.setContentsMargins(0, 0, 0, 0)
        layout.setMargin(0)
        layout.addWidget(widgets["logo"])

        layout = QtWidgets.QHBoxLayout(panels["body"])
        layout.setContentsMargins(0, 0, 0, 0)
        layout.setMargin(0)
        layout.addWidget(widgets["okBtn"])
        layout.addWidget(widgets["cancelBtn"])
        layout.addWidget(widgets["resetBtn"])

        layout = QtWidgets.QHBoxLayout(panels["body"])
        layout.setContentsMargins(0, 0, 0, 0)
        layout.setMargin(0)
        layout.addWidget(QtWidgets.QLabel("My Footer"))

        #  ___________________
        # |           |       |
        # |___________|       |
        # |           |       |
        # |___________|       |
        # |           |       |
        # |___________|_______|
        #
        layout = QtWidgets.QGridLayout(pages["home"])
        layout.setContentsMargins(0, 0, 0, 0)
        layout.addWidget(panels["header"], 0, 0)
        layout.addWidget(panels["body"], 1, 0)
        layout.addWidget(panels["footer"], 2, 0)
        layout.addWidget(panels["sidebar"], 0, 1, 0, 2)

Notes

• S20 Layouts MUST all be called layout; don't bother with unique names or maintaining reference to them
• S21 All layouts MUST be populated together, in the same block
• S22 Illustrations MAY be used to communicate intent and reduce cognitive load, especially for complex layouts

class Window(QtWidgets.QMainWindow):
    title = "My Window"

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

        ...

        widgets["logo"].setCursor(QtCore.Qt.PointingHandCursor)
        widgets["okBtn"].setTooltip("Press me")
        widgets["cancelBtn"].setTooltip("Don't press me")
        widgets["cancelBtn"].setIcon(icons["cancelIcon"])
        widgets["someView"].setModel(model.SomeModel())

        widgets["okBtn"].clicked.connect(self.on_okbtn_clicked)
        widgets["someView"].selectionChanged.connect(self.on_someview_selection_changed)

Notes

• S30 Widgets MUST be initialised together, in the same block
• S31 Widgets SHOULD be initialised in the order they were declared
• S32 Signals MUST be initialised together, in the same block
• S33 Signals MUST all have an on_ prefix, they are responding to an event
• S34 Models MUST be singletons, usable from both controller and tests

class Window(QtWidgets.QMainWindow):
    title = "My Window"

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

        ...

        self._pages = pages
        self._panels = panels
        self._widgets = widgets
        self._ctrl = ctrl

        self.setup_a()
        self.setup_b()
        self.update_c()

        # Misc
        # ...

Notes

• S40 Private members MUST be declared together, after initialisation
• S41 Post-initialisation MUST happen together

class Window(QtWidgets.QMainWindow):
    title = "My Window"

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

        ...

    def on_this(...):
    def on_that(...):
    def on_this_changed(...):
    def on_that_changed(...):
    ...

Notes

• S50 All methods of a window SHOULD be signal handlers, there isn't much else for a window to do

class Window(QtWidgets.QMainWindow):
    title = "My Window"

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

        ...

class SmallHelperWidget(...):
    pass

class SpecialityButton(...):
    pass

Notes

• S60 Single-use helper classes and functions for window SHOULD reside with window

Structure II - Controller

A view merely interprets what the user wants, the controller is what actually makes it happen. When a button is pressed, a signal handler calls on the controller to take action.

class View(Widget):
    ...

    def on_copy_clicked(self):
        self._ctrl.copy()

Likewise, when the controller does something, the view is what tells the user about it.

class Controller(QtCore.QObject):
    state_changed = QtCore.Signal(str)

class View(Widget):
    def __init__(self, ctrl, parent=None):
        super(View, self).__init__(parent)

        ...

        ctrl.state_changed.connect(self.on_state_changed)

    def on_state_changed(self, state):
        self._widgets["statusLabel"].setText(state)

• S60 The controller MAY operate freely, such as in response to IO, a timed event or signals from an external process or web request.
• S61 The view MAY access the controller
• S62 The controller MAY NOT access the view
• S63 The controller MAY be a singleton, in which case it doesn't need passing around to view or its widgets
• S64 The controller MAY be accessed by individual widgets. If you can, think of how QML allows this and how convenient and intuitive that is, without any apparent downside

Structure III - Model

...

Responsiveness

Always develop your application synchronously at first, blocking at every turn. Once a series of operations run stable but cause too much delay (200 ms+), consider parallelising and what should be happening from the users point of view.

1. What does the user need to know? E.g. progress bar, status message, email on completion. Your safest course of action is switching page to a "Loading Screen", then consider balancing between interactivity and information.
2. What is the user allowed to do? E.g. some buttons disabled, some actions able to operate in parallel. Your simplest course of action is to keep the GUI responsive, but disable all widgets. Then look for a balance between enabled and safe.

---

Heads up

This got long. I'll pause here and update as I go. Potentially turn it into a more formal document, like CONTRIBUTING.md. Is this a good idea? Does this kind of thing help? Does it limit more than it liberates? Let me know.

[davidlatwe]

Awesome post !

But I though the model was the brain ? The thing that handles the business logic, and the control is the nerves. Controls receive the user inputs and the model changes data by it's logic so the view represent model process result visually.

[mottosso]

But I though the model was the brain ?

I've honestly never considered that. Whenever I read up on the topic, I typically conclude with "nobody knows" because there are so many variations on MVC (MVV, MVP, MV, what else?) and I've never fully understood how they differ. I've always turned to whatever Qt does, as it's at least documented and wouldn't result in a framework-on-framework.

But a quick search just now took me to a post I've read once before.

• https://stackoverflow.com/questions/5543198/why-qt-is-misusing-model-view-terminology

Except today it makes sense.

One of the things I've always struggled with is where to store the model.

• If we instantiate it in the view, then the controller would not have access to it, for e.g. initialisation. If we instantiate it in the controller, then we create this indirect relationship between controller and view, where the view needs to reach into the controller for a model. Yuk.

So what ends up happening is the model is updated from the view, with data from the controller, which means it cannot be used/tested without a view, and that's bad.

Nuking the controller, and making the model the controller instead would make life a whole lot easier. The Qt docs even says so, which I honestly haven't noticed until just now; there is no C in MVC. I don't like the idea of mixing data with business logic; to me the model has always been just dumb "structured" data. Something facilitating a call to data() and setData(). And it feels a little backwards also having e.g. launchProcess() and printSummary() in there.

But let's try that, I think you're right.