4
\$\begingroup\$

Initially I got this code:

import sys
import os
import random

from PyQt5 import QtWidgets
from PyQt5 import QtGui
from PyQt5 import QtCore

random.seed(1)


class Model(QtCore.QObject):

    item_added = QtCore.pyqtSignal(int)
    item_removed = QtCore.pyqtSignal(int)

    def __init__(self):
        super().__init__()
        self.items = {}

    def add_item(self):
        guid = random.randint(0, 10000)
        new_item = {
            "pos": [random.randint(50, 100), random.randint(50, 100)]
        }
        self.items[guid] = new_item
        self.item_added.emit(guid)

    def remove_item(self):
        list_keys = list(self.items.keys())

        if len(list_keys) == 0:
            self.item_removed.emit(-1)
            return

        guid = random.choice(list_keys)
        self.item_removed.emit(guid)
        del self.items[guid]


class View1():

    def __init__(self, main_window):
        self.main_window = main_window

        view = QtWidgets.QGraphicsView()
        self.scene = QtWidgets.QGraphicsScene(None)
        self.scene.addText("Hello, world!")

        view.setScene(self.scene)
        view.setStyleSheet("background-color: red;")

        main_window.setCentralWidget(view)


class View2():

    add_item = QtCore.pyqtSignal(int)
    remove_item = QtCore.pyqtSignal(int)

    def __init__(self, main_window):
        self.main_window = main_window

        button_add = QtWidgets.QPushButton("Add")
        button_remove = QtWidgets.QPushButton("Remove")
        vbl = QtWidgets.QVBoxLayout()
        vbl.addWidget(button_add)
        vbl.addWidget(button_remove)
        view = QtWidgets.QWidget()
        view.setLayout(vbl)

        view_dock = QtWidgets.QDockWidget('View2', main_window)
        view_dock.setWidget(view)

        main_window.addDockWidget(QtCore.Qt.RightDockWidgetArea, view_dock)

        model = main_window.model
        button_add.clicked.connect(model.add_item)
        button_remove.clicked.connect(model.remove_item)


class Controller():

    def __init__(self, main_window):
        self.main_window = main_window

    def on_item_added(self, guid):
        view1 = self.main_window.view1
        model = self.main_window.model

        print("item guid={0} added".format(guid))
        item = model.items[guid]
        x, y = item["pos"]
        graphics_item = QtWidgets.QGraphicsEllipseItem(x, y, 60, 40)
        item["graphics_item"] = graphics_item
        view1.scene.addItem(graphics_item)

    def on_item_removed(self, guid):
        if guid < 0:
            print("global cache of items is empty")
        else:
            view1 = self.main_window.view1
            model = self.main_window.model

            item = model.items[guid]
            x, y = item["pos"]
            graphics_item = item["graphics_item"]
            view1.scene.removeItem(graphics_item)
            print("item guid={0} removed".format(guid))


class MainWindow(QtWidgets.QMainWindow):

    def __init__(self):
        super().__init__()

        # (M)odel ===> Model/Library containing should be UI agnostic, right now it's not
        self.model = Model()

        # (V)iew      ===> Coupled to UI
        self.view1 = View1(self)
        self.view2 = View2(self)

        # (C)ontroller ==> Coupled to UI
        self.controller = Controller(self)

        self.attach_views_to_model()

    def attach_views_to_model(self):
        self.model.item_added.connect(self.controller.on_item_added)
        self.model.item_removed.connect(self.controller.on_item_removed)


if __name__ == "__main__":
    app = QtWidgets.QApplication(sys.argv)

    form = MainWindow()
    form.setMinimumSize(800, 600)
    form.show()
    sys.exit(app.exec_())

Which is not following the best practices at all, ideally I'd like my business logic to be UI agnostic. Therefore, I've rewritten it as the code below, but I am still not happy with it.

import sys
import os
import random

from PyQt5 import QtWidgets
from PyQt5 import QtGui
from PyQt5 import QtCore

random.seed(1)


class ItemManager():

    def __init__(self):
        super().__init__()
        self.items = {}

    def add_random_item(self):
        guid = random.randint(0, 10000)
        new_item = {
            "pos": [random.randint(50, 100), random.randint(50, 100)]
        }
        self.items[guid] = new_item
        return guid

    def get_random_item(self):
        list_keys = list(self.items.keys())

        if len(list_keys) == 0:
            raise Exception("global cache is empty")
        else:
            guid = random.choice(list_keys)
            return guid

    def remove_item(self, guid):
        del self.items[guid]


class ModelPyqt(QtCore.QObject):
    item_added = QtCore.pyqtSignal(int)
    item_removed = QtCore.pyqtSignal(int)

    def __init__(self):
        super().__init__()
        self.item_manager = ItemManager()

    def add_random_item(self):
        guid = self.item_manager.add_random_item()
        self.item_added.emit(guid)

    def remove_random_item(self):
        try:
            guid = self.item_manager.get_random_item()
            self.item_removed.emit(guid)
            self.item_manager.remove_item(guid)
        except Exception as e:
            print(e)


class View1Pyqt():

    def __init__(self, main_window):
        self.main_window = main_window

        view = QtWidgets.QGraphicsView()
        self.scene = QtWidgets.QGraphicsScene(None)
        self.scene.addText("Hello, world!")

        view.setScene(self.scene)
        view.setStyleSheet("background-color: red;")

        main_window.setCentralWidget(view)


class View2Pyqt():

    add_item = QtCore.pyqtSignal(int)
    remove_item = QtCore.pyqtSignal(int)

    def __init__(self, main_window):
        self.main_window = main_window

        button_add = QtWidgets.QPushButton("Add")
        button_remove = QtWidgets.QPushButton("Remove")
        vbl = QtWidgets.QVBoxLayout()
        vbl.addWidget(button_add)
        vbl.addWidget(button_remove)
        view = QtWidgets.QWidget()
        view.setLayout(vbl)

        view_dock = QtWidgets.QDockWidget('View2', main_window)
        view_dock.setWidget(view)

        main_window.addDockWidget(QtCore.Qt.RightDockWidgetArea, view_dock)

        model = main_window.model
        button_add.clicked.connect(model.add_random_item)
        button_remove.clicked.connect(model.remove_random_item)


class ControllerPyqt():

    def __init__(self, main_window):
        self.main_window = main_window

    def on_item_added(self, guid):
        view1 = self.main_window.view1
        model = self.main_window.model

        print("item guid={0} added".format(guid))
        item = model.item_manager.items[guid]
        x, y = item["pos"]
        graphics_item = QtWidgets.QGraphicsEllipseItem(x, y, 60, 40)
        item["graphics_item"] = graphics_item
        view1.scene.addItem(graphics_item)

    def on_item_removed(self, guid):
        view1 = self.main_window.view1
        model = self.main_window.model

        item = model.item_manager.items[guid]
        x, y = item["pos"]
        graphics_item = item["graphics_item"]

        try:
            view1.scene.removeItem(graphics_item)
            print("item guid={0} removed".format(guid))
        except Exception as e:
            print(e)


class AppPyqt(QtWidgets.QMainWindow):

    def __init__(self):
        super().__init__()

        # (M)odel
        self.model = ModelPyqt()

        # (V)iew
        self.view1 = View1Pyqt(self)
        self.view2 = View2Pyqt(self)

        # (C)ontroller
        self.controller = ControllerPyqt(self)

        self.attach_views_to_model()

    def attach_views_to_model(self):
        self.model.item_added.connect(self.controller.on_item_added)
        self.model.item_removed.connect(self.controller.on_item_removed)


if __name__ == "__main__":
    app = QtWidgets.QApplication(sys.argv)

    form = AppPyqt()
    form.setMinimumSize(800, 600)
    form.show()
    sys.exit(app.exec_())

As you can see in the controller I'm modifying objects coming from the logic (UI agnostic) and adding some UI references like this item["graphics_item"] = graphics_item and graphics_item = item["graphics_item"].

How would you fix this bad design issue in the second dummy snippet?

\$\endgroup\$
3
  • \$\begingroup\$ Please note that dummy code should still be runnable and perform a (maybe trivial) function. ( i have not PyQt5 so I cannot test myself) \$\endgroup\$ – Caridorc Sep 17 '16 at 21:28
  • 1
    \$\begingroup\$ @Caridorc Well, in the past this maybe wouldn't be trivial/dummy code cos installing/compiling pyqt was quite time consuming. Nowadays it's justa a fact of having some python3 venv and doing pip install PyQt5, that will allow you to run this simple trivial example. My original question was actually broader than this but it wasn't getting enough answers (SO) so I moved to codereview and changed the format. I've though presenting some little example showing some of my main concerns about UI&logic SoC could be good enough to elaborate upon it \$\endgroup\$ – BPL Sep 17 '16 at 22:34
  • \$\begingroup\$ @Caridorc Here's my original question btw, as you can see, my main concern is about knowing how to decouple properly UI & business logic (no matter which framework you use) \$\endgroup\$ – BPL Sep 17 '16 at 22:43

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Browse other questions tagged or ask your own question.