Wo mache ich den Fehler?

[Palexer]

Hallo, ich fange gerade an mich in meiner Freizeit mit Python zu beschäftigen. Als Übung wollte ich mal ein Programm schreiben, welches einem zufällige Passwörter generiert. Die von mir als erstes angefertigte Version für das Terminal funktioniert ganz gut. Nun wollte ich mit PyQt5 eine GUI zu demselben Programm zu erstellen (teilweise auch mit QtDesigner). Nun kommt aber als Ausgabe im Terminal meist nur Whitespace oder meine eigene Fehlermeldung. Nun frage ich mich wo ich denn etwas falsch mache. Hier der Code:

für die GUI:

Code: Alles auswählen

from PyQt5 import QtCore, QtGui, QtWidgets
from def_global import Password 

class Ui_mainWindow(object):


    def __init__(self):
        self.letters = "n"
        self.digits = "n"
        self.punctuation = "n"


    def setupUi(self, mainWindow):


        mainWindow.setObjectName("mainWindow")
        mainWindow.resize(652, 422)
        self.centralwidget = QtWidgets.QWidget(mainWindow)
        self.centralwidget.setObjectName("centralwidget")
        self.label_top = QtWidgets.QLabel(self.centralwidget)
        self.label_top.setGeometry(QtCore.QRect(10, 20, 161, 16))
        self.label_top.setObjectName("label_top")
        self.spin_box_num = QtWidgets.QSpinBox(self.centralwidget)
        self.spin_box_num.setGeometry(QtCore.QRect(10, 240, 65, 32))
        self.spin_box_num.setMinimum(1)
        self.spin_box_num.setProperty("value", 10)
        self.spin_box_num.setObjectName("spin_box_num")
        self.check_letters = QtWidgets.QCheckBox(self.centralwidget)
        self.check_letters.setGeometry(QtCore.QRect(10, 100, 111, 22))
        self.check_letters.setObjectName("check_letters")
        self.check_digits = QtWidgets.QCheckBox(self.centralwidget)
        self.check_digits.setGeometry(QtCore.QRect(10, 130, 101, 22))
        self.check_digits.setObjectName("check_digits")
        self.check_punctuation = QtWidgets.QCheckBox(self.centralwidget)
        self.check_punctuation.setGeometry(QtCore.QRect(10, 160, 111, 22))
        self.check_punctuation.setObjectName("check_punctuation")
        self.label_num_chars = QtWidgets.QLabel(self.centralwidget)
        self.label_num_chars.setGeometry(QtCore.QRect(10, 210, 151, 20))
        self.label_num_chars.setObjectName("label_num_chars")
        self.btn_create = QtWidgets.QPushButton(self.centralwidget)
        self.btn_create.setGeometry(QtCore.QRect(550, 360, 88, 34))
        self.btn_create.setObjectName("btn_create")
        self.label_sel_chars = QtWidgets.QLabel(self.centralwidget)
        self.label_sel_chars.setGeometry(QtCore.QRect(10, 60, 311, 18))
        self.label_sel_chars.setAcceptDrops(False)
        self.label_sel_chars.setObjectName("label_sel_chars")
        self.label_password = QtWidgets.QLabel(self.centralwidget)
        self.label_password.setGeometry(QtCore.QRect(10, 310, 621, 18))
        self.label_password.setText("")
        self.label_password.setObjectName("label_password")
        mainWindow.setCentralWidget(self.centralwidget)
        self.statusbar = QtWidgets.QStatusBar(mainWindow)
        self.statusbar.setObjectName("statusbar")
        mainWindow.setStatusBar(self.statusbar)

        self.retranslateUi(mainWindow)
        QtCore.QMetaObject.connectSlotsByName(mainWindow)

        self.char_num = self.spin_box_num.value
        

        self.btn_create.clicked.connect(lambda _: self.create_password(self.char_num))

    def retranslateUi(self, mainWindow):

        _translate = QtCore.QCoreApplication.translate
        mainWindow.setWindowTitle(_translate("mainWindow", "Password Generator"))
        self.label_top.setText(_translate("mainWindow", "Create a new passoword."))
        self.check_letters.setText(_translate("mainWindow", "Letters"))
        self.check_digits.setText(_translate("mainWindow", "Digits"))
        self.check_punctuation.setText(_translate("mainWindow", "Punctuation"))
        self.label_num_chars.setText(_translate("mainWindow", "Number of characters:"))
        self.btn_create.setText(_translate("mainWindow", "Create"))
        self.label_sel_chars.setText(_translate("mainWindow", "Select the characters the password should contain."))



    def create_password(self, char_num):
        """Creates the password"""
        if self.check_letters.isChecked():
            self.letters = "y"
        
        if self.check_letters.isChecked():
            self.digits = "y"
        
        if self.check_punctuation.isChecked():
            self.punctuation = "y"
        

        password = Password(char_num, self.letters, self.digits, self.punctuation)
        print(password)


if __name__ == "__main__":
    import sys
    app = QtWidgets.QApplication(sys.argv)
    mainWindow = QtWidgets.QMainWindow()
    ui = Ui_mainWindow()
    ui.setupUi(mainWindow)
    mainWindow.show()
    sys.exit(app.exec_())

und hier für die Passwort-Funktion:

Code: Alles auswählen

import secrets, string

class Password:
    def __init__(self, char_num:int=10, letters:str = "n", digits:str="n", punctuation:str = "n"):
        self.char_num = char_num
        self.password_list = []
        self.password = ""
        self.chars = []


        if letters == "y":
            for char in string.ascii_letters:
                self.chars.append(char)
        
        if digits == "y":
            for char in string.digits:
                self.chars.append(char)

        if punctuation == "y":
            for char in string.punctuation:
                self.chars.append(char)

        if letters != "y" and digits != "y" and punctuation != "y":
            print("Error. You need to add at least one type of characters to your password.")
            exit()


    def create(self) -> str:
        """Creates the password"""

        for char in range(self.char_num):
            char = secrets.choice(self.chars)
            self.password_list.append(char)

        self.password = "".join(self.password_list)

        return self.password

    def __repr__(self):
        return self.password

[__blackjack__]

@Palexer: Mal davon abgesehen das man keinen GUI-Code mehr generiert, hatte der generierte Code oben einen Kommentar der davor warnt die Datei zu verändern. Verwende das `PyQt5.uic`-Modul um die ``*.ui``-Datei zur Laufzeit zu laden, oder schreib ein eigenes Modul, welches das Modul mit dem generierten Code importiert und verwendet, aber schreibe nichts in den generierten Code.

Der generierte Code zeigt lauter absolute Positions- und Grössenangaben: Das funktioniert nicht. Das sieht auf Deinem Rechner, beziehungsweise dem auf dem Du das so entworfen hast richtig aus, aber wird auf anderen Rechnern mit anderen Bildschirmauflösungen und Systemeinstellung Probleme machen, bis hin zur unbenutzbarkeit wenn sich Widgets so überlappen das sie nicht mehr vernüntig verwendet werden können.

`self.letters`, `self.digits`, und `self.punctuation` sollten keine Attribute sein. Ausserdem ist der Code so geschrieben, dass der Benutzer die auf "y" setzen kann, aber nie wieder auf "n". Zudem sollten die nicht "y" und "n" als Werte haben, denn dafür hat Python einen eigenen Datentyp: `bool` mit den Werten `True` und `False`.

`def_global` ist kein guter Modulname.

Die `Password`-Klasse ist keine Klasse, das ist einfach eine Funktion die sehr umständlich geschrieben ist.

Womit prüfst Du die Typannotationen? Falls die Antwort auf die Frage „Hä?“, „Gar nicht.“, oder „Manchmal, von Hand mit xy“ sein sollte, dann lass den Unsinn bitte einfach weg.

Genau wie beim GUI-Code: Wahrheitswerte sind `True` und `False`, nicht "y" und "n".

Die `__repr__()`-Methode ist für eine eindeutige Darstellung für Programmierer gedacht. Konventionell ist das entweder etwas das man als Python-Quelltext ansehen kann, und das ausgeführt wieder ein gleiches Objekt erzeugt, oder aber die Darstellung ist in spitze Klammern eingefasst ("< … >") und fängt üblicherweise nach dem "<" mit dem Datentypnamen an. Damit ein Programmierer bei der Ausgabe weiss was er da vor sich hat. Das was Du da implementiert hast, eine Zeichenkettendarstellung für einen Benutzer, gehört in die `__str__()`-Methode.

Wenn das denn eine Klasse wäre, und nicht nur eine verkleidete, aufgeplusterte Funktion, dann würde mindestens `password_list` nicht zum Zustand eines solchen Objekts gehören. Und `password` auch nicht, denn das ein Attribut gesetzt *und* als Rückgabewert von einer Methode geliefert wird, ist ein „code smell“.

Anstatt die Argumente zu prüfen ob mindestens eine Zeichenklasse gewählt wurde, würde ich das prüfen was wichtig ist um weiter zu machen: ob `chars` Zeichen enthält oder nicht.

Eine `print()`-Ausgabe und `exit()` hat dort nichts zu suchen. Die `print()`-Ausgabe sieht der GUI-Benutzer nicht, und das eine Funktion der Programmlogik einfach so das gesamte Programm beendet ist mehr als ungünstig. In dem Falle möchte man doch lieber dem Benutzer eine Fehlermeldung anzeigen.

Bei ``for char in …`` ist das `char` falsch und sollte eigentlich auch von der statischen Typprüfung angemeckert werden, denn es wäre dann ja gleichzeitig ein Namen für ganze Zahlen und für Zeichenketten, ohne dass das irgendwo explizit so deklariert/annotiert wäre. Was ja auch keinen Sinn machen würde. Namen die nicht verwendet werden, aber aus syntaktischen Gründen im Code stehen müssen, werden per Konvention einfach `_` genannt.

`password_list` und `char` sind aber auch überflüssig. Man muss nicht jeden kleinen Trippelschritt an einen Namen binden. Grunddatentypen gehören auch nicht in Namen.

Namen sollte man nicht kryptisch abkürzen. Ich würde `characters` schreiben wenn das gemeint ist und nicht `chars`.

Die gesamte Klasse schrumpft letztlich auf folgende kleine Funktion zusammen (ungetestet):

Code: Alles auswählen

import secrets
import string
from itertools import compress


def create_password(length, use_letters, use_digits, use_punctuation):
    if length < 0:
        raise ValueError("length must be positive")
    
    characters = "".join(
        compress(
            [string.ascii_letters, string.digits, string.punctuation],
            [use_letters, use_digits, use_punctuation],
        )
    )
    if not characters:
        raise ValueError(
            "You need to add at least one type of characters to your password."
        )

    return "".join(secrets.choice(characters) for _ in range(length))

[Palexer]

@Palexer: Mal davon abgesehen das man keinen GUI-Code mehr generiert, hatte der generierte Code oben einen Kommentar der davor warnt die Datei zu verändern. Verwende das `PyQt5.uic`-Modul um die ``*.ui``-Datei zur Laufzeit zu laden, oder schreib ein eigenes Modul, welches das Modul mit dem generierten Code importiert und verwendet, aber schreibe nichts in den generierten Code.

Der generierte Code zeigt lauter absolute Positions- und Grössenangaben: Das funktioniert nicht. Das sieht auf Deinem Rechner, beziehungsweise dem auf dem Du das so entworfen hast richtig aus, aber wird auf anderen Rechnern mit anderen Bildschirmauflösungen und Systemeinstellung Probleme machen, bis hin zur unbenutzbarkeit wenn sich Widgets so überlappen das sie nicht mehr vernüntig verwendet werden können.

`self.letters`, `self.digits`, und `self.punctuation` sollten keine Attribute sein. Ausserdem ist der Code so geschrieben, dass der Benutzer die auf "y" setzen kann, aber nie wieder auf "n". Zudem sollten die nicht "y" und "n" als Werte haben, denn dafür hat Python einen eigenen Datentyp: `bool` mit den Werten `True` und `False`.

`def_global` ist kein guter Modulname.

Die `Password`-Klasse ist keine Klasse, das ist einfach eine Funktion die sehr umständlich geschrieben ist.

Womit prüfst Du die Typannotationen? Falls die Antwort auf die Frage „Hä?“, „Gar nicht.“, oder „Manchmal, von Hand mit xy“ sein sollte, dann lass den Unsinn bitte einfach weg.

Genau wie beim GUI-Code: Wahrheitswerte sind `True` und `False`, nicht "y" und "n".

Die `__repr__()`-Methode ist für eine eindeutige Darstellung für Programmierer gedacht. Konventionell ist das entweder etwas das man als Python-Quelltext ansehen kann, und das ausgeführt wieder ein gleiches Objekt erzeugt, oder aber die Darstellung ist in spitze Klammern eingefasst ("< … >") und fängt üblicherweise nach dem "<" mit dem Datentypnamen an. Damit ein Programmierer bei der Ausgabe weiss was er da vor sich hat. Das was Du da implementiert hast, eine Zeichenkettendarstellung für einen Benutzer, gehört in die `__str__()`-Methode.

Wenn das denn eine Klasse wäre, und nicht nur eine verkleidete, aufgeplusterte Funktion, dann würde mindestens `password_list` nicht zum Zustand eines solchen Objekts gehören. Und `password` auch nicht, denn das ein Attribut gesetzt *und* als Rückgabewert von einer Methode geliefert wird, ist ein „code smell“.


Anstatt die Argumente zu prüfen ob mindestens eine Zeichenklasse gewählt wurde, würde ich das prüfen was wichtig ist um weiter zu machen: ob `chars` Zeichen enthält oder nicht.

Eine `print()`-Ausgabe und `exit()` hat dort nichts zu suchen. Die `print()`-Ausgabe sieht der GUI-Benutzer nicht, und das eine Funktion der Programmlogik einfach so das gesamte Programm beendet ist mehr als ungünstig. In dem Falle möchte man doch lieber dem Benutzer eine Fehlermeldung anzeigen.

Bei ``for char in …`` ist das `char` falsch und sollte eigentlich auch von der statischen Typprüfung angemeckert werden, denn es wäre dann ja gleichzeitig ein Namen für ganze Zahlen und für Zeichenketten, ohne dass das irgendwo explizit so deklariert/annotiert wäre. Was ja auch keinen Sinn machen würde. Namen die nicht verwendet werden, aber aus syntaktischen Gründen im Code stehen müssen, werden per Konvention einfach `_` genannt.

`password_list` und `char` sind aber auch überflüssig. Man muss nicht jeden kleinen Trippelschritt an einen Namen binden. Grunddatentypen gehören auch nicht in Namen.

Namen sollte man nicht kryptisch abkürzen. Ich würde `characters` schreiben wenn das gemeint ist und nicht `chars`.

Die gesamte Klasse schrumpft letztlich auf folgende kleine Funktion zusammen (ungetestet):

Code: Alles auswählen

import secrets
import string
from itertools import compress


def create_password(length, use_letters, use_digits, use_punctuation):
    if length < 0:
        raise ValueError("length must be positive")
    
    characters = "".join(
        compress(
            [string.ascii_letters, string.digits, string.punctuation],
            [use_letters, use_digits, use_punctuation],
        )
    )
    if not characters:
        raise ValueError(
            "You need to add at least one type of characters to your password."
        )

    return "".join(secrets.choice(characters) for _ in range(length))

Viele Dank für die sehr ausführliche Hilfe. Ich habe wohl noch einiges zu lernen.

[__deets__]

Auf die Liste gehoert auch noch, nicht den gesamten Text des Posts davor zu zitieren. Danke.