mysqldb Objekt

[mobby]

Hey Leute,

ich bin begeisteter RaspberryPi-Bastler und hantiere in dem Zusammenhang viel mit MySQL-Datenbanken. Als Schnittstelle zur Datenbank verwende ich das [url=http://mysql-python.sourceforge.net/MyS ... qldb-Modul[/url]. Um mit den Umgang mit dem Modul zu erleichtern habe ich mir ein eigenes Skript geschrieben, in dem ich ein Objekt für den schnellen Datenbankeintrag gebaut habe. Dieses Skript würde ich gerne der RaspberryPi-Community zur Verfügung stellen. Bevor das geschieht wollte ich es hier allerdings nochmal prüfen lassen, ob man etwas beser machen kann. Mit besser meine ich keinen schöneren Programmierstil, sondern wirklich essentielle Dinge, die aus funktionalen Gründen ergänzt bzw. geändert werden sollten.

Ich bin mir darüber im klaren, dass ich noch ein wenig mehr Error-Handling betreiben muss, aber verstehe nicht ganz wo und wie ich da rangehen soll. Z.B. "KeyboardInterrupt" abfangen hat nicht wirklich funtkioniert. Weiß aber nicht warum. Hier auf jeden Fall mal der Code. Vielen Dank für Feedback!

Code: Alles auswählen

# -*- coding: utf-8 -*- 


import sys
import time
import MySQLdb as db
from contextlib import closing


class Connector(object):
    def __init__(self, db_host, db_user, db_pw, db_name, db_table, *db_rows):
        self.db_host = db_host
        self.db_user = db_user
        self.db_pw = db_pw
        self.db_name = db_name
        self.db_table = db_table
        self.db_rows = db_rows
        self.sql_str1 = ""
        self.sql_str2 = ""
        
        for x in range (len(self.db_rows)):
            if x == (len(self.db_rows) -1 ):
                self.sql_str1 = self.sql_str1 + self.db_rows[x]
                self.sql_str2 = self.sql_str2 + "%s"
            else:
                self.sql_str1 = self.sql_str1 + self.db_rows[x] + ", "
                self.sql_str2 = self.sql_str2 + "%s, "

    def write_into_db(self, *values):
        self.values = values
        self.sql = "INSERT INTO " + self.db_table + " (" + self.sql_str1 + \
            ") VALUES (" + self.sql_str2 + ")"
        try:
            with closing(
                db.connect(
                    self.db_host,
                    self.db_user, 
                    self.db_pw, 
                    self.db_name
                )
            ) as connection:
                connection.cursor().execute(self.sql, self.values)
                connection.commit()
            print "Input successfull"
        except db.Error as e:
            mysql_error = "A MySQL error occurred.\nDate: " + \
                time.strftime("%d-%m-%Y") + "\nTime: " + \
                time.strftime("%H:%M:%S") + "\nError code: " + \
                str(e.args[0]) + "\n" + "Error message: " + str(e.args[1])
            print mysql_error
            with open('logfile.txt', 'a') as file:
                file.write(mysql_error)
                file.write("\n\n---------------\n\n")
                file.close
        except:
            print "An unexpected error occurred:", sys.exc_info()[0]


def main():
    value1 = "teststring"
    value2 = 10.3
    value3 = 10
    value4 = True
    
    test = Connector("192.168.42.1", "mysqlexcel", "8bef566e3a961cf3aeaec93c75260dd4da7466b7e248d134f6b021c64161f62c",
                 "energie", "test", "row1", "row2", "row3", "row4")
                 
    test.write_into_db(value1, value2, value3, value4)


if __name__ == '__main__':
    main()

[BlackJack]

@mobby: Ganz essentiell würde ich das komplett wegwerfen. Klingt vielleicht ein bisschen hart aber da sind so viele unschöne Sachen drin, und es gibt SQLAlchemy.

Sternchenmagie bei Argumenten sollte man nur machen wenn man sie tatsächlich braucht. `db_rows` hätte man auch ohne Sternchen schreiben können, und eine Sequenz als Argument übergeben. Dann sieht man beim Aufruf auch besser wo die *Spalten*namen anfangen. Spalte heisst auf englisch `column` nicht `row`.

``for x in range (len(self.db_rows))`` ist in Python ein „anti pattern”. Man kann direkt über Elemente von Sequenzen iterieren, ohne den Umweg über einen Index machen zu müssen. Und für die Zeichenketten suchst Du die `join()`-Methode, also zum Beispiel ``', '.join(column_names)`` um eine Zeichenkette mit den Spaltennamen getrennt durch Kommas zu erstellen.

Bei `write_into_db()` gibt's wieder das Sternchen beim Argument. Das ist hier ungünstig weil die Daten in der Regel in irgendeiner Form als Struktur/Sequenz kommen werden. Da braucht man dann auch beim Aufruf immer das Sternchen, statt einfach das Objekt direkt zu übergeben.

Warum werden die Werte an das Objekt gebunden? Die werden da nie wieder abgefragt, das Attribut wird ausserhalb der `__init__()` eingeführt, und so wirklich zum Zustand eine Connector-Objekts gehören die auch nicht.

Die Fehlerbehandlung ist keine. Du brauchst nicht *mehr* Fehlerbehandlung sondern eindeutig *weniger* die Fehler einfach verschluckt und das Programm in einem vielleicht inkonsistenten Zustand einfach weiterlaufen lässt. Für Logs gibt es ausserdem das `logging`-Modul.

[BlackJack]

@mobby: Nachtrag: Man könnte auch die Klasse an sich in Frage stellen weil Klassen die nur aus einer `__init__()` und einer einzigen Methode bestehen, oft ein Hinweis darauf sind, dass man nicht wirklich eine Klasse benötigt sondern auch mit einer Funktion und `partial()` oder einem einfachen Closure auskommt.

`db_rows` müsste auch kein Attribut sein, genau so wenig wie das in der einzigen Methode eingeführte `sql`. Den Wert dafür hätte man auch schon in der `__init__()` erstellen können, denn der ändert sich ja nicht mehr.

Das zusammensetzen von Zeichenketten und Werten mittels ``+`` ist ziemlich unübersichtlich. Zeichenkettenformatierung, wo man die endgültige Zeichenkette viel besser erkennen kann weil die Platzhalter in der Zeichenkette stehen und nicht Ausdrücke und literale Stückchen sich abwechseln, ist viel lesbarer.

Das ganze ohne Klasse (ungetestet):

Code: Alles auswählen

from contextlib import closing
from itertools import repeat

import MySQLdb as db
 

def create_insert_function(
    host, username, password, db_name, table_name, column_names
):
    sql = 'INSERT INTO {0} ({1}) VALUES ({2})'.format(
        table_name,
        ', '.join(column_names),
        ', '.join(repeat('%s', len(column_names)))
    )
    
    def insert_function(values):
        with closing(
            db.connect(host, username, password, db_name)
        ) as connection:
            connection.cursor().execute(sql, values)
            connection.commit()

    return insert_function

 
def main():
    values = ('teststring', 10.3, 10, True)
   
    insert = create_insert_function(
        '192.168.42.1',
        'mysqlexcel',
        '8bef566e3a961cf3aeaec93c75260dd4da7466b7e248d134f6b021c64161f62c',
        'energie',
        'test',
        ('row1', 'row2', 'row3', 'row4')
    )
    insert(values)
 
 
if __name__ == '__main__':
    main()

Ich würde aber trotzdem SQLAlchemy vorziehen.

[mobby]

@BlackJack: Ich muss gestehen, dass ich nach deiner ersten Antwort etwas frustriert war. Nicht nur über dein Kommentar, welches vollkommen angebracht war, sondern vor allem über viele offensichtliche Fehler, die mir vorher einfach nicht aufgefallen sind. Mit einigen deiner Kommentare konnte ich direkt etwas anfangen, mit anderen weniger. Auf jeden Fall würde ich mich gerne für die Code-Alternative bedanken. Es ist doch wirklich erstaunlich wie einfach manche Sachen sind. Man müsste nur wissen, wie man das selber anwendet . In dem Sinne danke ich dir nicht ¬nur für den Code sondern auch für die Gelegenheit etwas dazuzulernen.

Gruß
mobby

[mobby]

//Nachtrag:

Falls ich diese jetzt wunderbar arbeitende Funktion um Methoden wie z.B. Einträge löschen oder editieren ergänzen möchte, bin ich wieder auf eine Klasse angewiesen oder sehe ich das falsch? Thx 4 feedback!

[BlackJack]

@mobby: Dann wäre eine Klasse nötig, ja. Aber wie gesagt, ich würde das nicht selber schreiben, beziehungsweise nicht von Null. Es gibt ja SQLAlchemy.

Code: Alles auswählen

#!/usr/bin/env python
from sqlalchemy import create_engine, MetaData


def main():
    db_user = 'mysqlexcel'
    db_password = (
        '8bef566e3a961cf3aeaec93c75260dd4da7466b7e248d134f6b021c64161f62c'
    )
    engine = create_engine(
        'mysql+mysqldb://{0}:{1}@192.168.42.1/energie?charset=utf-8'.format(
            db_user, db_password
        )
    )
    metadata = MetaData(engine, reflect=True)
    test_table = metadata.tables['test']

    test_table.insert().values(
        row1='teststring', row2=10.3, row3=10, row4=True
    ).execute()

    test_table.update().where(test_table.c.id == 1).values(row3=42).execute()

    for row in test_table.select().where(test_table.c.row4 == True).execute():
        print row


if __name__ == '__main__':
    main()

Das funktioniert mit verschiedenen DBMS und DB-Modulen, die Tabellenobjekte kann man wie in dem Beispiel aus einer bestehenden Datenbank per „reflection” erstellen lassen, oder selber definieren. Letzteres bietet sich an wenn die Tabellen in der Datenbank (potentiell) noch nicht existieren, weil man sie dann auch gleich aus der Definition der Tabellenobjekte erstellen lassen kann.

Und SQLAlchemy bietet auch ein „Object Relational Mapper”-Package, wenn man die Datenbankzeilen gerne als Objekte verwenden und möglichst weniger mit SQL zu tun haben möchte.