QTableView Daten editieren

[HeinKurz]

Hi!
Ich hab eine App mit einem QTableView. Ich möchte die Daten direkt im QTable editieren und die Änderungen sofort in die Datenbank schreiben. Nachfolgend meine Lösung (nur der entscheidende Teil im TableModel) die so weit auch funktioniert aber ich habe den Verdacht, dass das keine sehr elegante Lösung ist. Wie würdet ihr es machen?

Code: Alles auswählen

class TableModel(QtCore.QAbstractTableModel):
    def __init__(self, data):
        super(TableModel, self).__init__()
        self._data = data
        #self.rows_nr, self.columns_nr = data.shape
        self.hheaders = ["ID", "Datum", "Konto", "Betrag", "Bmkg", "Geschaeft"]

    def data(self, index, role=Qt.DisplayRole):
        # ...................

    def setData(self, index, value, role=Qt.EditRole):
        # editieren
        if index.isValid() and role == Qt.EditRole:
            # self._data ist Tuple und lässt sich nicht editieren und tmp=list(self._data) funktioniert nicht, daher ...
            # aber das geht sicher besser/einfacher !?!?!?
            tmp = []
            for i in range(0, len(self._data)):
                t = []
                for j in range(0, len(self._data[i])):
                    t.append(self._data[i][j])
                tmp.append(t)
            tmp[index.row()][index.column()] = value
            self._data = tmp
            #self._data[index.row()][index.column()] = value   # funktioniert nicht
            self.dataChanged.emit(index, index)   # ??????
            # Daten prüfen
            fehler = False
            if index.column() == 1:   # Spalte Datum
                if len(value) != 10:
                    fehler = True
                else:
                    if value[4:5] != "-" or value[7:8] != "-":
                        fehler = True
            if index.column() == 2:   # Spalte Konto
                value = value.upper()
                if len(value) != 1 or (value != "G" and value != "S" and value != "B"):
                    fehler = True
            if index.column() == 3:   # Spalte Betrag
                val = str(value).replace(",", ".")
                if len(val) < 1:
                    fehler = True
            if index.column() == 4:   # Spalte Bmkg
                if len(value) < 1:
                    fehler = True
            if fehler == True:
                winsound.PlaySound("SystemExclamation", winsound.SND_ALIAS)
                #QMessageBox.critical(self, "Fehler", "Fehler bei der Dateneingabe.")
                return False
            # speichern in der DB
            id = self._data[index.row()][0]
            f = ["", "Datum", "Konto", "Betrag", "Bmkg", "Geschaeft"]
            sqls = "UPDATE KontoPW SET " + f[index.column()] + "="
            if index.column() != 3:
                sqls += "'" + value + "'"
            else:
                sqls += val
            sqls += " WHERE ID=" + str(id)
            file = cwd + "\\Konto.db"
            # check if DB-file exists
            if not os.path.isfile(file):
                #QMessageBox.critical(self, "Fehler", "Die Datei\r\n   " + file + "\r\nfehlt.\r\nDas Programm wird beendet.")
                winsound.PlaySound("SystemExclamation", winsound.SND_ALIAS)
                return False
            # connect
            conn = sqlite3.connect(file)
            cur = conn.cursor()
            cur.execute(sqls)
            conn.commit()
            conn.close()
            #self.ShowData()
            return True
        return False

    def rowCount(self, index):
        # ...................

[__blackjack__]

@HeinKurz: Der Kommentar das `self._data()` ein Tupel ist, riecht komisch. Wenn da ein Tupel nicht funktioniert, dann sollte das halt auch keines sein und an entprechender Stelle *einmal* in eine Liste umgewandelt werden, und nicht jedes mal eine neue Kopie angelegt werden wenn ein Wert gesetzt wird.

Die Schleifen mit den Indexwerte sind auch überflüssig und auch `t` und letztlich auch `tmp`, denn natürlich geht das auch mit `list()`:

Code: Alles auswählen

            tmp = []
            for i in range(0, len(self._data)):
                t = []
                for j in range(0, len(self._data[i])):
                    t.append(self._data[i][j])
                tmp.append(t)
            self._data = tmp
            
            # =>
            
            self._data = list(map(list, self._data))

Das setzen des Wertes in `data` passiert an der falschen Stelle. Der Rückgabewert signalisiert ja ob das setzen erfolgreich war, der Code setzt aber den Wert und prüft danach erst ob das überhaupt passieren darf.

Das heisst wenn man das nicht ständig im Code wiederholen möchte, dann darf es da nicht so viele ``return True`` geben.

Die Spaltennummern schliessen sich ja gegenseitig aus, also sind das keine ``if`` sondern ``elif``.

Statt `value` in drei Teilbedingungen gegen Buchstaben zu testen, würde man das mit *einem* Ausdruck und ``in`` machen.

Der Test bei Spalte 1 testet nicht wirklich ob das ein gültiges Datum ist und bei Spalte 3 nicht wirklich ob das eine Zahl ist.

Man macht keine Vergleiche mit literalen Wahrheitswerten. Bei dem Vergleich kommt doch nur wieder ein Wahrheitswert bei heraus. Entweder der, den man sowieso schon hatte; dann kann man den auch gleich nehmen. Oder das Gegenteil davon; dafür gibt es ``not``.

`id` ist der Name einer eingebauten Funktion, den sollte man nicht an etwas anderes binden.

Das zusammenbasteln von Werten in SQL-Abfragen mittels Zeichenkettenoperationen ist im besten Fall unperformant oder funktioniert nicht für alle Werte und im schlechtesten Fall eine riesige Sicherheitslücke über die der Programmbenutzer beliebiges SQL ausführen kann. Zum Beispiel über die Bemerkungsspalte.

`file` ist kein guter Name für einen Datei*namen*. Bei etwas das `file` heisst. erwartet der Leser Methoden wie `read()` oder `write()` und `close()`.

`cwd` kommt aus dem nichts. Und falls der Name das bedeutet was er üblicherweise bedeutet, macht es auch nicht wirklich Sinn das vor den relativen Dateinamen zu setzen, denn zu dem Pfad ist der Name ja sowieso schon relativ.

Der Pfadtrenner im Namen ist keine gute Idee und zusammenbasteln von Pfadteilen mit Zeichenkettenoperationen erst recht nicht. Dafür gibt es das `pathlib`-Modul.

Den Cursor sollte man auch schliessen und das am besten bei Verbindung und Cursor mit `contextlib.closing()` und ``with`` sicherstellen.

Der Datenbankdateiname und der Test tauchen doch sicher noch an anderer Stelle im Programm als Kopie(n) auf. Das sollte nicht sein.

Das mit `winsound` ist unschön weil es damit auf Windows beschränkt ist.

Nicht ganz äquivalenter Zwischenstand:

Code: Alles auswählen

    def setData(self, index, value, role=Qt.EditRole):
        if index.isValid() and role == Qt.EditRole:
            if (
                (
                    index.column() == 1
                    and len(value) == 10
                    and value[4] == value[7] == "-"
                )
                or (index.column() == 2 and value in {"G", "S", "B"})
                or (index.column() == 3 and value)
                or (index.column() == 4 and value)
            ):
                db_file_path = Path("Konto.db")
                if not db_file_path.is_file():
                    winsound.PlaySound("SystemExclamation", winsound.SND_ALIAS)
                    return False

                column_name = ["datum", "konto", "betrag", "bmkg"][
                    index.column() - 1
                ]
                with closing(sqlite3.connect(db_file_path)) as connection:
                    with closing(connection.cursor()) as cursor:
                        cursor.execute(
                            (
                                f"UPDATE kontopw SET {column_name} = %s"
                                f" WHERE id = %s"
                            ),
                            value,
                            self._data[index.row()][0],
                        )
                    connection.commit()

                self._data[index.row()][index.column()] = value
                self.dataChanged.emit(index, index)

                return True

            else:
                winsound.PlaySound("SystemExclamation", winsound.SND_ALIAS)

        return False

Allerdings würde ich die Prüfungen so auch nicht im Model abladen sondern schon in der GUI dafür sorgen, dass man in der Datumsspalte nur Datumsangaben eingeben kann, und wenn es nur drei mögliche Werte gibt, dann sollte das über ein Dropdown gelöst werden über das man nur diese drei Werte eingeben kann, und so weiter. Qt hat für so etwas ja schon fertige Eingabeelemente und/oder zusätzlich QValidator.

[HeinKurz]

Vielen Dank __blackjack__ für die super Tipps.