class MainWindow(QMainWindow):
def connect_signals_slots(self) -> None:
# define parameters of devices (i.e. ports)
# and corresponding GUI elements
# ports hardcoded for illustration purposes only
self.device_ports = [
'COM1',
'COM2',
'COM3',
'COM7',
'COM10',
]
self.btns = [
self.ui.btn1,
self.ui.btn2,
self.ui.btn3,
self.ui.btn4,
self.ui.btn5,
]
self.spinboxes = [
self.ui.spinbox1,
self.ui.spinbox2,
self.ui.spinbox3,
self.ui.spinbox4,
self.ui.spinbox5,
]
# connect signals to slots
for x in self.btns:
x.clicked.connect(self.on_btn_click)
for x in self.spinboxes:
x.editingFinished.connect(self.on_spinbox)
def get_device_port(self, element: QWidget, list: List[Any]) -> str:
if element not in list:
raise ValueError(f'Element {element} not found in the given list')
idx = list.index(element)
port = self.device_ports[idx]
return port
def on_btn_click(self):
port = self.get_device_port(self.sender(), self.btns)
self.controller.do_stuff(port)
def on_spinbox(self, value):
port = self.get_device_port(self.sender(), self.spinboxes)
self.controller.change_value(port, value)
Public I use this approach when a Qt GUI has a set of control elements that repeats several times to control several different entities. E.g. in this example, I have a set of a button + a spinbox repeated 5 times to control 5 different external devices connected to different COM ports. I find it convenient to connect all the signals from e.g. buttons to one single slot, and then to find out what button was pressed by looking for `self.sender()` in the list of buttons. To make it more universal, I use a `get_device_port()` method to let the user look for the sender in any given list - i.e. in the list of buttons when the button is pressed or in the list of spinboxes when a spinbox value is updated. But is this approach safe and scalable enough?
Never overwrite built-in names, like list, sum or max - if later someone wants to use built-in function, he will find it replaced with your variable. If you really need to use one of those names, append underscore: sum_ = 15.
This code is not really needed or may be simplified
list.index(element) will raise itself if element is not there, no need to handle that case manually
Type hints help humans and linters (like mypy) to understand what to expect "in" and "out" for a function. Not only it serves as a documentation for others (and you after some time, when the code is wiped from your "brain cache"), but also allows using automated tools to find type errors.
Seems like things could be organized in a better way.
This approach is understandable, and the code provided here is clean and easy to read. However, in my personal opinion, here all elements are "mixed" (i.e. you have a list of ports, list of buttons, and list of spinboxes), and you have some logic to pick elements in these lists. I would "group" related elements instead. For example, I would create a function that would create the controls group for any port and connect it to appropriate handlers. Compared to your solution, the code is not scattered across the class but is just inside one function.
Suggested change:def create_controls_for_port(self, port: str):
button = Button(...)
button.clicked.connect(lambda: self.controller.do_stuff(port))
spinbox = Spinbox(...)
spinbox.editingFinished.connect(lambda value: self.controller.change_value(port, value))
Create new review request