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)
\$\endgroup\$
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.
\$\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\$ – riskypenguin May 31 at 9:12

Your Answer

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

Not the answer you're looking for? Browse other questions tagged or ask your own question.