2
\$\begingroup\$

This module is the main GUI for a project I'm working on. It's not yet final, but I feel I've become somewhat blind to potential improvements / refactorings, but I'm sure there are plenty. The module started out as a mess, as I generated the code with QtDesigner. The code was neither readable nor maintainable (it's probably not meant to be), and I feel it's come a long way since then. Most text and error messages are in German, but they should be relatively irrelevant for functionality.

This is my first time using PyQt, so I'm thankful for all types of feedback. I've intentionally left out the other project modules, as the code's already quite long and most of the functionality is completely seperate from the GUI. If a code snippet isn't clear without knowing the other modules I'd say it isn't relevant for this review. I'm mainly looking for feedback on the code structure. The one functionality still left to implement is securely storing and using the password entered by the user. Right now it's all plaintext. I'm thinking about a seperate prompt using getpass or similiar, any advice is welcome.

import sys
import logging
from typing import (Type, Callable)
from pathlib import Path
from io import StringIO
from contextlib import (redirect_stdout, redirect_stderr)
from PyQt5 import QtGui
from PyQt5.QtWidgets import *
from PyQt5.QtCore import (QMetaObject, QRect)
from src.Backend import config_handler
from src.Backend.Executors.EmailAnalyzer import EmailAnalyzer
from src.Backend.Objects.exceptions import MessageException
from PATHS import (ROOT_DIR, ICONS_DIR, FONTS_DIR)
log = logging.getLogger(__name__)
MAIN_GUI = None
def show() -> None:
 """ Creates and runs an instance of UIMainWindow """
 global MAIN_GUI
 # do nothing if MAIN_GUI is still running
 if MAIN_GUI is not None:
 return
 app = QApplication(sys.argv)
 MAIN_GUI = UIMainWindow()
 MAIN_GUI.show()
 app.exec_()
 MAIN_GUI = None
def get_date_list() -> list[str]:
 """Opens a QDialog box that requests a list of dates from the user"""
 if MAIN_GUI is None:
 raise RuntimeError("get_date_list cannot be called before running Main GUI")
 # only import DateListGUI if it's actually needed
 from src.Frontend.DateListGUI import DateListGUI
 dialog = DateListGUI(parent=MAIN_GUI)
 dialog.show()
 dialog.exec_()
 return dialog.datelist
# noinspection PyAttributeOutsideInit
class UIMainWindow(QMainWindow):
 def __init__(self):
 super().__init__()
 self.width = 1000
 self.border_margin = 10
 self.widget_width = self.width - 2 * self.border_margin
 # vertical distance between sections
 self.section_margin = 30
 self.horizontal_separator_height = int(self.section_margin * (2 / 3))
 # UI is vertically arranged
 self.row_height = 37 # height for each row
 # rows may have a text label that describes functionality / purpose
 self.label_width = 20 # width for each label
 icon_path = ICONS_DIR.joinpath("IconGUI.ico")
 if icon_path.exists():
 self.setWindowIcon(QtGui.QIcon(icon_path.as_posix()))
 self.setup_fonts()
 self.setFont(self.font_regular)
 self.setup_ui()
 @staticmethod
 def load_fonts(font_files: list[str]) -> None:
 """ Loads fonts from the filesystem to QFontDatabase """
 for ttf_file in font_files:
 font_path = FONTS_DIR.joinpath(ttf_file)
 QtGui.QFontDatabase().addApplicationFont(font_path.as_posix())
 def setup_fonts(self) -> None:
 """ Sets attributes for regular, bold and italic font variations """
 font_files = ["OpenSans-Regular.ttf", "OpenSans-Bold.ttf", "OpenSans-Italic.ttf"]
 self.load_fonts(font_files)
 # Only font_regular is currently in use
 self.font_regular = QtGui.QFont("Open Sans")
 self.font_bold = QtGui.QFont("Open Sans")
 self.font_bold.setBold(True)
 self.font_italic = QtGui.QFont("Open Sans")
 self.font_italic.setItalic(True)
 def setup_ui(self) -> None:
 """ Creates basic UI layout and widgets """
 # UI is created in rows from top to bottom
 # current_y keeps track of how far down we've come for dynamic placement
 current_y = self.border_margin
 if self.objectName():
 self.setObjectName(u"main_window")
 self.centralwidget = QWidget(self)
 self.centralwidget.setObjectName(u"centralwidget")
 self.setCentralWidget(self.centralwidget)
 # ---------- FORM LAYOUT FOR E-MAIL SERVER DATA ----------
 # form layout, will be passed as parent to all contained widgets
 self.widget_server_data = QWidget(self.centralwidget)
 self.widget_server_data.setObjectName(u"widget_server_data")
 self.widget_server_data.setGeometry(
 QRect(self.border_margin, current_y, self.widget_width, self.row_height * 4))
 self.form_layout_data = QFormLayout(self.widget_server_data)
 self.form_layout_data.setObjectName(u"form_layout_data")
 self.form_layout_data.setContentsMargins(0, 0, 0, 0)
 # create widgets for form
 self.create_form_widget(self.widget_server_data, self.form_layout_data, QLabel, "label_base_path", 0,
 QFormLayout.LabelRole)
 self.create_form_widget(self.widget_server_data, self.form_layout_data, QLabel, "textlabel_base_path", 0,
 QFormLayout.FieldRole)
 self.create_form_widget(self.widget_server_data, self.form_layout_data, QLabel, "label_email_address", 1,
 QFormLayout.LabelRole)
 self.create_form_widget(self.widget_server_data, self.form_layout_data, QLineEdit,
 "textedit_email_address", 1, QFormLayout.FieldRole)
 self.create_form_widget(self.widget_server_data, self.form_layout_data, QLabel, "label_password", 2,
 QFormLayout.LabelRole)
 self.create_form_widget(self.widget_server_data, self.form_layout_data, QLineEdit, "textedit_password", 2,
 QFormLayout.FieldRole)
 self.create_form_widget(self.widget_server_data, self.form_layout_data, QCheckBox,
 "checkbox_gui_save_password", 3, QFormLayout.FieldRole)
 # ---------- HORIZONTAL SEPARATOR ----------
 current_y += self.widget_server_data.height() + 10
 self.place_horizontal_line(parent=self.centralwidget, number=1,
 pos=(self.border_margin, current_y),
 size=(self.widget_width, self.horizontal_separator_height))
 # ---------- FORM LAYOUT FOR MAIN OUTPUT ----------
 current_y += self.section_margin
 # form layout, will be passed as parent to all contained widgets
 self.widget_main_output = QWidget(self.centralwidget)
 self.widget_main_output.setObjectName(u"widget_main_output")
 self.widget_main_output.setGeometry(
 QRect(self.border_margin, current_y, self.widget_width, self.row_height * 2))
 self.form_layout_main_output = QFormLayout(self.widget_main_output)
 self.form_layout_main_output.setObjectName(u"form_layout_main_output")
 self.form_layout_main_output.setContentsMargins(0, 0, 0, 0)
 # create widgets for form
 self.create_form_widget(self.widget_main_output, self.form_layout_main_output, QLabel,
 "label_main_output_directory", 1, QFormLayout.LabelRole)
 self.create_form_widget(self.widget_main_output, self.form_layout_main_output, QLineEdit,
 "textedit_main_output_directory", 1, QFormLayout.FieldRole)
 self.create_form_widget(self.widget_main_output, self.form_layout_main_output, QPushButton,
 "pushbutton_main_output_directory", 2, QFormLayout.FieldRole)
 # ---------- HORIZONTAL SEPARATOR ----------
 current_y += self.widget_main_output.height() + 10
 self.place_horizontal_line(parent=self.centralwidget, number=2,
 pos=(self.border_margin, current_y),
 size=(self.widget_width, self.horizontal_separator_height))
 # ---------- FORM LAYOUT FOR WRITING EMAILS TO FILE ----------
 current_y += self.section_margin
 # form layout, will be passed as parent to all contained widgets
 self.widget_email_output = QWidget(self.centralwidget)
 self.widget_email_output.setObjectName(u"widget_email_output")
 self.widget_email_output.setGeometry(
 QRect(self.border_margin, current_y, self.widget_width, self.row_height * 3))
 self.form_layout_email_output = QFormLayout(self.widget_email_output)
 self.form_layout_email_output.setObjectName(u"form_layout_email_output")
 self.form_layout_email_output.setContentsMargins(0, 0, 0, 0)
 # create widgets for form
 self.create_form_widget(self.widget_email_output, self.form_layout_email_output, QLabel,
 "label_email_output", 0, QFormLayout.LabelRole)
 self.create_form_widget(self.widget_email_output, self.form_layout_email_output, QCheckBox,
 "checkbox_email_write_to_file", 0, QFormLayout.FieldRole)
 self.create_form_widget(self.widget_email_output, self.form_layout_email_output, QLabel,
 "label_email_output_directory", 1, QFormLayout.LabelRole)
 self.create_form_widget(self.widget_email_output, self.form_layout_email_output, QLineEdit,
 "textedit_email_output_directory", 1, QFormLayout.FieldRole)
 self.create_form_widget(self.widget_email_output, self.form_layout_email_output, QPushButton,
 "pushbutton_email_output_directory", 2, QFormLayout.FieldRole)
 # ---------- HORIZONTAL SEPARATOR ----------
 current_y += self.widget_email_output.height() + 10
 self.place_horizontal_line(parent=self.centralwidget, number=3,
 pos=(self.border_margin, current_y),
 size=(self.widget_width, self.horizontal_separator_height))
 # ---------- VERTICAL LAYOUT FOR MISC SETTINGS ----------
 current_y += self.section_margin
 # vertical layout, will be passed as parent to all contained widgets
 self.widget_options = QWidget(self.centralwidget)
 self.widget_options.setObjectName(u"widget_options")
 self.widget_options.setGeometry(
 QRect(self.border_margin, current_y, self.widget_width, self.row_height * 3))
 self.vertical_layout_options = QVBoxLayout(self.widget_options)
 self.vertical_layout_options.setObjectName(u"vertical_layout_options")
 self.vertical_layout_options.setContentsMargins(0, 0, 0, 0)
 # create widgets for vertical layout
 self.create_layout_widget(self.widget_options, self.vertical_layout_options, QCheckBox,
 "checkbox_email_mark_read")
 self.create_layout_widget(self.widget_options, self.vertical_layout_options, QCheckBox,
 "checkbox_output_print_timeinfo")
 self.create_layout_widget(self.widget_options, self.vertical_layout_options, QCheckBox,
 "checkbox_output_print_form_counts")
 # ---------- HORIZONTAL SEPARATOR ----------
 current_y += self.widget_options.height() + 10
 self.place_horizontal_line(parent=self.centralwidget, number=5,
 pos=(self.border_margin, current_y),
 size=(self.widget_width, self.horizontal_separator_height))
 # ---------- MAIN PROCEDURE BUTTONS ----------
 button_width = int(self.widget_width / 2 - self.border_margin)
 current_y += self.section_margin
 self.place_pushbutton(parent=self.centralwidget, name="pushbutton_analyse_forms",
 pos=(self.border_margin, current_y),
 size=(button_width, 31))
 self.place_pushbutton(parent=self.centralwidget, name="pushbutton_quit",
 pos=(self.border_margin * 2 + button_width, current_y),
 size=(button_width, 31))
 # Main Window
 self.setCentralWidget(self.centralwidget)
 self.menubar = QMenuBar(self)
 self.menubar.setObjectName(u"menubar")
 self.menubar.setGeometry(QRect(0, 0, self.width, 22))
 self.setMenuBar(self.menubar)
 self.statusbar = QStatusBar(self)
 self.statusbar.setObjectName(u"statusbar")
 self.setStatusBar(self.statusbar)
 self.setup_widgets()
 QMetaObject.connectSlotsByName(self)
 self.resize(self.width, current_y + (self.border_margin * 7))
 def place_horizontal_line(self, parent: QWidget, number: int,
 pos: tuple[int, int], size: tuple[int, int]) -> None:
 """ Places a horizontal separator line on the parent widget """
 line = QFrame(parent)
 line_name = f"horizontal_line{number}"
 line.setObjectName(line_name)
 line.setGeometry(QRect(*pos, *size))
 line.setFrameShape(QFrame.HLine)
 line.setFrameShadow(QFrame.Sunken)
 # probably not necessary, since horizontal lines are never changed
 setattr(self, line_name, line)
 def place_pushbutton(self, parent: QWidget, name: str,
 pos: tuple[int, int], size: tuple[int, int]) -> None:
 """ Places a QPushButton line on the parent widget """
 button = QPushButton(parent)
 button.setObjectName(name)
 button.setGeometry(QRect(*pos, *size))
 setattr(self, name, button)
 def create_form_widget(self, parent_widget: QWidget, parent_form: QFormLayout, widget_type: Type[QWidget],
 widget_name: str, form_index: int, form_role: int) -> None:
 """ Creates and places a widget of widget_type on parent_form """
 widget = widget_type(parent_widget)
 widget.setObjectName(widget_name)
 parent_form.setWidget(form_index, form_role, widget)
 setattr(self, widget_name, widget)
 def create_layout_widget(self, parent_widget: QWidget, parent_layout: QVBoxLayout,
 widget_type: Type[QWidget], widget_name: str) -> None:
 """ Creates and places a widget of widget_type on parent_form """
 widget = widget_type(parent_widget)
 widget.setObjectName(widget_name)
 parent_layout.addWidget(widget)
 setattr(self, widget_name, widget)
 def setup_widgets(self) -> None:
 """ Sets text and connects functionality for all widgets on the main UI """
 self.setWindowTitle("CLIENT_NAME - E-Mail Formular-Auswertung")
 # FORM LAYOUT --> E-MAIL SERVER DATA
 self.label_base_path.setText("Aktueller Ordner:".ljust(self.label_width))
 self.label_base_path.setFont(self.font_bold)
 self.textlabel_base_path.setText(ROOT_DIR.as_posix())
 self.label_email_address.setText("E-Mail Adresse:".ljust(self.label_width))
 self.label_email_address.setFont(self.font_bold)
 self.label_password.setText("Passwort:".ljust(self.label_width))
 self.label_password.setFont(self.font_bold)
 self.connect_widget_to_setting(self.textedit_email_address, "server_data", "email_address")
 self.connect_widget_to_setting(self.textedit_password, "server_data", "password")
 # FORM LAYOUT --> MAIN OUTPUT
 self.label_main_output_directory.setText("Ausgabe-Ordner:".ljust(self.label_width))
 self.label_main_output_directory.setFont(self.font_bold)
 self.connect_widget_to_setting(self.textedit_main_output_directory, "output", "directory")
 self.textedit_main_output_directory.textChanged.connect(lambda t: self.replace_on_textchange(t, "/", "\\"))
 self.setup_pushbutton(self.pushbutton_main_output_directory, "Ausgabe-Ordner für Formular-Auswertungen ändern",
 lambda: self.get_and_set_directory(self.textedit_main_output_directory))
 # FORM LAYOUT --> WRITE EMAIL TO FILE
 self.label_email_output.setText("E-Mail Ablage:".ljust(self.label_width))
 self.label_email_output.setFont(self.font_bold)
 self.checkbox_email_write_to_file.setText("Ausgewertete E-Mails as .txt-Dateien ablegen")
 self.connect_widget_to_setting(self.checkbox_email_write_to_file, "handle_emails", "write_to_file")
 self.label_email_output_directory.setText("Ablage-Ordner:".ljust(self.label_width))
 self.label_email_output_directory.setFont(self.font_bold)
 self.connect_widget_to_setting(self.textedit_email_output_directory, "handle_emails", "directory")
 self.textedit_email_output_directory.textChanged.connect(lambda t: self.replace_on_textchange(t, "/", "\\"))
 self.setup_pushbutton(self.pushbutton_email_output_directory, "Ablage-Ordner für E-Mails ändern",
 lambda: self.get_and_set_directory(self.textedit_email_output_directory))
 # VERTICAL LAYOUT --> SETTINGS
 self.checkbox_gui_save_password.setText("Passwort speichern")
 self.connect_widget_to_setting(self.checkbox_gui_save_password, "gui", "save_password")
 self.checkbox_email_mark_read.setText("Ausgewertete E-Mails als gelesen markieren")
 self.connect_widget_to_setting(self.checkbox_email_mark_read, "handle_emails", "mark_read")
 self.checkbox_output_print_timeinfo.setText("Prozedur-Laufzeiten ausgeben")
 self.connect_widget_to_setting(self.checkbox_output_print_timeinfo, "output", "print_timeinfo")
 self.checkbox_output_print_form_counts.setText("Anzahl der ausgewerteten Formulare nach Formular-Typ ausgeben")
 self.connect_widget_to_setting(self.checkbox_output_print_form_counts, "output", "print_form_counts")
 self.setup_pushbutton(self.pushbutton_analyse_forms, "Formulare aus E-Mail-Postfach auswerten",
 self.analyse_forms)
 self.setup_pushbutton(self.pushbutton_quit, "Programm beenden", self.quit)
 def replace_on_textchange(self, text: str, to_replace: str, replace_with: str) -> None:
 """ Replaces certain user inputs while typing in a QLineEdit or QTextEdit """
 sender = self.sender()
 if isinstance(sender, (QLineEdit, QTextEdit)):
 sender.setText(text.replace(to_replace, replace_with))
 @staticmethod
 def update_widget_from_setting(widget: QWidget, *config_keys: str) -> None:
 """ Reads setting from config_handler and writes value to QWidget """
 if isinstance(widget, QCheckBox):
 config_value = config_handler.get_setting(*config_keys, default=False)
 widget.setChecked(config_value)
 elif isinstance(widget, QLineEdit):
 config_value = config_handler.get_setting(*config_keys, default="")
 widget.setText(str(config_value))
 elif isinstance(widget, QTextEdit):
 config_value = config_handler.get_setting(*config_keys, default="")
 widget.setText(str(config_value))
 def connect_widget_to_setting(self, widget: QWidget, *config_keys: str) -> None:
 """ Connects Qwidget to update a config_handler setting on every change """
 self.update_widget_from_setting(widget, *config_keys)
 if isinstance(widget, QCheckBox):
 widget.clicked.connect(lambda c: self.update_setting_from_widget(c, *config_keys))
 elif isinstance(widget, QLineEdit):
 widget.textChanged.connect(lambda t: self.update_setting_from_widget(t, *config_keys))
 elif isinstance(widget, QTextEdit):
 widget.textChanged.connect(lambda t: self.update_setting_from_widget(t, *config_keys))
 @staticmethod
 def update_setting_from_widget(value: (str, bool), *config_keys: str) -> None:
 """ Wrapper for setting settings through config_handler """
 config_handler.set_setting(value, *config_keys)
 @staticmethod
 def setup_pushbutton(pushbutton: QPushButton, text: str, click_function: Callable) -> None:
 """ Sets up text and click functionality for QPushButton """
 pushbutton.setText(text)
 pushbutton.clicked.connect(click_function)
 def get_and_set_directory(self, target_lineedit: QLineEdit) -> None:
 """ Opens a QFileDialog and writes selected directory to target_lineedit """
 sender = self.sender() # sender is a QPushButton
 sender.setEnabled(False)
 try:
 initial_directory = Path(target_lineedit.text())
 if not initial_directory.exists():
 initial_directory = ROOT_DIR
 user_directory = str(
 QFileDialog.getExistingDirectory(self, "Ordner auswählen", initial_directory.as_posix()))
 if user_directory:
 target_lineedit.setText(user_directory)
 except Exception as e:
 log.exception("Unerwarteter Fehler")
 print(e.__str__())
 sender.setEnabled(True)
 def analyse_forms(self) -> None:
 """ Main functionality. Calls EmailAnalyzer.main and shows output in a QMessageBox """
 # sender is QPushButton
 sender = self.sender()
 sender.setEnabled(False)
 self.statusBar().showMessage("E-Mail Auswertung läuft...")
 self.statusBar().repaint()
 message_box = MyMessageBox(self)
 message_box.setStandardButtons(QMessageBox.Ok)
 message_box.setDefaultButton(QMessageBox.Ok)
 output = StringIO()
 with redirect_stdout(new_target=output), redirect_stderr(new_target=output):
 try:
 EmailAnalyzer().main()
 message_box.setWindowTitle("E-Mail-Auswertung")
 message_box.setText(output.getvalue())
 message_box.setIcon(QMessageBox.Information)
 except MessageException as e:
 log.exception("Erwarteter Fehler")
 message_box.setWindowTitle("Fehler")
 message_box.setText(e.message)
 message_box.setIcon(QMessageBox.Critical)
 except Exception as e:
 log.exception("Unerwarteter Fehler")
 message_box.setWindowTitle("Unerwarteter Fehler")
 message_box.setText(f"Unerwarteter Fehler:\n"
 f"{e.__class__}\n"
 f"{e.__str__()}\n\n"
 f"Bitte die Logs an den Entwickler schicken.")
 message_box.setIcon(QMessageBox.Critical)
 self.statusBar().showMessage("Auswertung abgeschlossen")
 self.statusBar().repaint()
 message_box.exec_()
 self.statusBar().showMessage("Auswertung abgeschlossen", msecs=4000)
 sender.setEnabled(True)
 def closeEvent(self, event) -> None:
 """ Redirects the PyQt close event to custom quit function. """
 self.quit()
 def quit(self) -> None:
 """ Check if password needs to be cleared, then close the GUI """
 if config_handler.get_setting("gui", "save_password", default=False) is False:
 config_handler.set_setting("", "server_data", "password")
 super().close()
class MyMessageBox(QMessageBox):
 """ Custom MessageBox that sets its width at every event """
 # This is a hacky workaround to be able to set the box size, which PyQt doesn't easily support
 # Based on:
 # https://stackoverflow.com/questions/2655354/how-to-allow-resizing-of-qmessagebox-in-pyqt4/2664019#2664019
 # Basically, every event is caught and the width is set to self.fixedWidth
 # self.fixedWidth is updated every time the text changes (usually only during initialization)
 def __init__(self, parent):
 super().__init__(parent)
 self.fixed_width = 750
 def event(self, e):
 result = super().event(e)
 self.setFixedWidth(self.fixed_width)
 return result
 def setText(self, a0: str) -> None:
 longest_line = max(a0.split("\n"), key=len)
 # These magic numbers are the result of manual trial and error
 # I found them to match the text width the most accurately, but it's far from perfect
 width = self.fontMetrics().boundingRect(longest_line).width()
 self.fixed_width = max(750, round(width * 1.4))
 super().setText(a0)
asked May 30, 2021 at 18:11
\$\endgroup\$

1 Answer 1

1
\$\begingroup\$

A random collection of things:

  • As with many other syntactic contexts, import (Type, Callable) does not require explicit tuple notation, and such notation is only useful if the imports are so long as to span more than one line
  • Generally want to avoid import *, even if it means a fairly long list of imports. Or maybe better, make a convenience alias for the module and keep everything in that namespace, like import PyQt5.QtWidgets as qw
  • Non-reentrant global MAIN_GUI is a smell, and will impede testing. show is at the top level and should own it, passing it down as necessary
  • only import DateListGUI if it's actually needed - why? Is this to avoid a known circular import, performance reasons, or other? Usually this should not be done.
  • Under setup_ui, if ever you find yourself writing a # -------- partition between blocks of code, that's a big hint that you should divide the function into subroutines
  • It's confusing that you accept multiple config_keys for config_handler.get_setting(*config_keys - maybe the documentation already explains that this is nested dictionary traversal, but if it doesn't it should. Also maybe use key_path for the associated argument name?
  • print(e.__str__()) should just be equivalent to print(e)
  • These magic numbers are the result of manual trial and error is, as you already know, suspicious. What are the input and output measurement units? Making this explicit would help.
answered May 30, 2021 at 21:01
\$\endgroup\$
1
  • \$\begingroup\$ Good points, I'll have to implement most of them and especially look to breaking down setup_ui further. For reference: config_handler.get_setting accepts an arbitrary number of config keys because my project config consists of nested dictionaries written to a json file. The old version of config_handler and an example of config.json can be found in a previous question. Not sure if that's even close to a best practice, that just seemed the most intuitive to me. \$\endgroup\$ Commented May 31, 2021 at 9:12

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

By clicking "Post Your Answer", you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.