This document shares some basic code review of the SnowUI latest branches, mostly focusing on architecture and model usage in the plugins.
The plugin system looks solid. There are some risks associated with this approach, since importing something runs the code in the module there can be side-effects of loading plugins at startup. Since this is happening in the main thread, a bad plugin can crash the application/prevent other plugins from loading/do something nefarious. The security elements are more different to resolve, but you can load plugins in a separate process, or at least validate them in a separate process first.
Not sure this is really worth it, it’ll depend if you’re expect users to create/share plugins.
The plugins all have models, but they don’t seem to be functional/it’s not clear if they’re really useful as models. There a few things I’d suggest here.
In the plugin files there are bits of code like the following. This “reaching into” other objects to attach behavior on is a bit of a code smell.
self.ctrls._ui.btn_add_db.clicked.connect(self.add_db)
The way I would implement something like this is have a separate controller/manager that handles the “add_db” functionality (this could be the plugin.py file you have already, or use the plugin as a wrapper and add a separate core.py) and have the UI connect to that by importing it, e.g.
from . import plugin
self.btn_add_db.clicked.connect(plugin.add_db)
That’s not possible now because the plugin holds a reference of the view. That’s a problem. The business logic shouldn’t need to know anything about the UI. Looking in another plugin, we have something like this, e.g.
self.ctrls._ui.btn_load.clicked.connect(
lambda: self.load(self.explr._ui.treeWidget.selectedItems())
)
In this case the handler needs to have access to the UI to get the selectItems, but also needs to know the structure of the UI, the name of elements in the UI etc. — again, this is a code smell. That data (the selected items) should preferably be available through some kind of model. That allows you to keep the business logic separate from the UI.
In the latest update of my book I’ve added some simple MVC-like stuff based on Python dictionaries. I copied an example below to give you an example. This model is completely generic, so you could feasibly re-use it for all the plugins. I think this might be what you’re beginning to implement with the DataManager
import sys
import random
from collections import UserDict
from PyQt5.QtWidgets import (
QApplication,
QComboBox,
QFormLayout,
QLineEdit,
QMainWindow,
QSpinBox,
QWidget,
QPushButton,
QCheckBox
)
from PyQt5.QtCore import pyqtSignal, QObject
class DataModelSignals(QObject):
# Emit an "updated" signal when a property changes.
updated = pyqtSignal()
class DataModel(UserDict):
def __init__(self, *args, **kwargs):
self.signals = DataModelSignals()
super().__init__(*args, **kwargs)
def __setitem__(self, key, value):
previous = self.get(key) # Get the existing value.
super().__setitem__(key, value) # Set the value.
if value != previous: # There is a change.
self.signals.updated.emit() # Emit the signal.
print(self) # Show the current state.
model = DataModel(
name="Johnina Smith",
age=10,
favorite_icecream='Vanilla',
disable_details=False,
)
class Controller:
""" Simple controller, which handles backups and other operations. """
backups = []
def capitalize(self):
model['name'] = model['name'].upper()
def store_backup(self):
self.backups.append(model.copy())
def restore_backup(self):
if not self.backups:
return # Ignore if empty.
# Randomly get a backup.
random.shuffle(self.backups)
backup = self.backups.pop() # Remove a backup.
model.update(backup) # Overwrite the data in the model.
def apply_title_case(self):
model['name'] = model['name'].title()
controller = Controller()
class MainWindow(QMainWindow):
def __init__(self):
super().__init__()
self.setWindowTitle("My App")
layout = QFormLayout()
# Dictionary to store the form data, with default data.
self.name = QLineEdit()
self.name.textChanged.connect(self.handle_name_changed)
self.age = QSpinBox()
self.age.setRange(0, 200)
self.age.valueChanged.connect(self.handle_age_changed)
self.icecream = QComboBox()
self.icecream.addItems(["Vanilla", "Strawberry", "Chocolate"])
self.icecream.currentTextChanged.connect(self.handle_icecream_changed)
self.title_btn = QPushButton("Set title case")
self.title_btn.pressed.connect(controller.apply_title_case)
# tag::connect[]
self.save_btn = QPushButton("Save")
self.save_btn.pressed.connect(controller.store_backup)
self.restore_btn = QPushButton("Restore")
self.restore_btn.pressed.connect(controller.restore_backup)
# end::connect[]
self.disable_details = QCheckBox("Disable details?")
self.disable_details.toggled.connect(self.handle_disable_details)
layout.addRow("Name", self.name)
layout.addRow(self.title_btn)
layout.addRow("Age", self.age)
layout.addRow("Favorite Ice cream", self.icecream)
layout.addWidget(self.disable_details) # QCheckBox has it's own label.
layout.addRow(self.save_btn)
layout.addRow(self.restore_btn)
widget = QWidget()
widget.setLayout(layout)
self.setCentralWidget(widget)
self.update_ui()
# Hook our UI sync into the model updated signal.
model.signals.updated.connect(self.update_ui)
def update_ui(self):
""" Synchronise the UI with the current model state. """
self.name.setText(model["name"])
self.age.setValue(model["age"])
self.icecream.setCurrentText(model["favorite_icecream"])
self.disable_details.setChecked(model['disable_details'])
# Enable/disable fields based on the disable_details state.
# disable_details is True/False, setting setDisabled to True disables the field.
self.age.setDisabled(model['disable_details'])
self.icecream.setDisabled(model['disable_details'])
def handle_name_changed(self, name):
model["name"] = name
def handle_age_changed(self, age):
model["age"] = age
def handle_icecream_changed(self, icecream):
model["favorite_icecream"] = icecream
def handle_disable_details(self, checked):
model["disable_details"] = checked
app = QApplication(sys.argv)
window = MainWindow()
window.show()
app.exec_()
The idea is to get a data-flow that completely uniform/predictable: the model is the single source of truth, changes in the UI update the model & the model updates are synced to the UI. Other code can read the model to get the current state, and also be notified from the model if something is changed. Not everything has to go through the model of course, pushing a button should still just trigger a behaviour. It’s just that if any state needs to be stored, doing this in the model makes the code simpler everywhere else.