@Odin: Dieses `clear()` für den Cursor ist hochgradig suspekt. So etwas braucht man nicht. Weg damit.
Die Namenskonvention bei Python wäre `add_tag()` statt `addTag()`. Siehe
Style Guide for Python Code.
Bei der ersten `query` gehören keine ' um den Platzhalter. Falls das tatsächlich funktionieren sollte ist das Zufall.
``not`` ist ein Operator und keine Funktion, das sollte man also auch nicht so schreiben als wäre es eine. Das gleiche gilt für ``if``. Die Klammern gehören da jeweils nicht hin und falls man Teilausdrücke klammern muss, dann gehört zwischen Schlüsselwort und Klammer ein Leerzeichen, eben damit das nicht wie ein Aufruf aussieht.
Das `with_rows`-Attribut gibt es allgemein auf Cursor-Objekten nicht. Das scheint eine Spezialität von dem Modul zu sein welches Du da verwendest. So etwas sollte man nicht machen. Einfach den Datensatz holen und auf die Ausnahme reagieren.
Wobei ich an der Stelle die Logik nicht ganz verstehe: Du testest ob es `name` als Tag gibt und wenn das der Fall ist, dann machst Du weiter. Sollte das nicht umgekehrt sein? Das weitergemacht wird falls es `name` als Tag noch *nicht* gibt?
Das mit der `iduebertag` (besser `id_uebertag` oder `uebertag_id`) und 'root' kann man verständlicher ausdrücken. Wobei ich mich gerade frage ob 'root' nicht als Datensatz in der Datenbank vorliegt oder warum man das hier gesondert behandeln muss, und das dann im Programm mit einem Wert der eine gültige ID in der Datenbank sein kann. Die werden ja wahrscheinlich von der Datenbank automatisch vergeben, und die könnte doch auch irgendwann einmal eine -1 vergeben. *Hoffst* Du einfach dass das niemals passiert und diese ID dann zwei Bedeutungen hat? Die die in der DB steht und die von der das Programm ausgeht. Ich würde ja 'root' einfach mit als Datensatz in der Datenbank aufnehmen.
Auf der linken Seite einer Zuweisung Klammern um einen einzelnen Namen zu setzen macht Null Sinn.
`ordner` sollte bitte schon als boole'scher Typ in die Funktion hinein kommen. So etwas wandelt man so früh wie möglich in passende Datentypen um und nicht irgendwo innerhalb von Funktionen die den Wert nicht selber in irgendeiner Weise von ”aussen” bekommen haben oder dediziert zum parsen gedacht sind.
Werte sollte man niemals selber in Zeichenketten mit SQL hineinformatieren. Das ist eine Sicherheitslücke und ein potentieller Performance-Verlust weil die Datenbank dann keine Ablaufpläne mit der SQL-Anfrage als Schlüssel cachen kann. Du machst das bei den ersten beiden Anfragen doch richtig, warum bei der dritten nicht mehr?
Zuguterletzt muss man dann nur noch diese komischen magischen Zahlen als Rückgabewerte loswerden. So etwas macht man in modernen Programmiersprachen mit Ausnahmen nicht mehr. Die Funktion läuft entweder erfolgreich durch oder löst eine Ausnahme aus. Mit dem Text bei der Ausnahme wird dann auch gleich deutlich wo das Problem liegt, statt kryptische Werte wie -1 oder -2 interpretieren zu müssen.
Ich lande dann bei so etwas (ungetestet):
Code: Alles auswählen
def add_tag(cursor, name, uebertag, ordner):
query = 'SELECT ID FROM Tags WHERE Name = %s;'
cursor.execute(query, name)
try:
cursor.fetchone()
except mysql.connector.Error:
try:
cursor.execute(query, uebertag)
except mysql.connector.Error:
raise ValueError('parent tag {0!r} does not exist'.format(uebertag))
else:
uebertag_id = cursor.fetchone()
cursor.execute(
'INSERT INTO Tags (Name, Uebertag, Ordner) VALUES (%s, %s, %s);',
(name, uebertag_id, ordner)
)
else:
raise ValueError('tag {0!r} already exists'.format(name))
Wobei ich persönlich ja mittlerweile für alles was mit relationalen Datenbanken zu tun hat SQLAlchemy als Abstraktionsschicht verwende. Das ORM macht vieles einfacher. Selbst wenn man kein ORM verwenden möchte kann man auch SQL einfacher und von der konkreten Datenbank unabhängiger programmatisch erstellen ohne mit Zeichenkettenoperationen SQL zusammenbasteln zu müssen.
Die Tabelle `Tags` hätte ich `Tag` genannt, denn so eine Tabellendefinition ist ja so ähnlich wie eine Klassendefinition: die beschreibt die Eigenschaften von *einem* Exemplar oder im Falle der Datenbank von *einem* Datensatz.
Die `Ubertag`-Spalte würde ich `uebertag_id` nennen, damit man am Namen schon sieht das es sich höchstwahrscheinlich um einen Fremdschlüssel handelt.
)
)