Elegantere Lösung finden

[peddy]

Hallo,

ich möchte User-Daten aus einem AD lesen und in eine SQL Tabelle übertragen. Ich habe jetzt etwas Quelltext zusammen gezimmert und funktional geht es so weit. Jedoch bin ich nicht wirklich glücklich über die Lösung des Problems. Daher würde ich mich sehr über Anregungen von erfahrenen Python Programmieren freuen.

Den Teil der AD Abfrage habe ich entfernt, damit es Übersichtlicher wird. Einen Beispiel-User habe ich in der Variable results hinterlegt. Der Code ist aktuell nicht kommentiert. Ich bitte das zu entschuldigen. Im Moment werden auch noch nicht alle Einträge im Dictionary verarbeitet, was aber noch kommen soll.

Code: Alles auswählen

#!/usr/bin/env python
# -*- coding: UTF-8 -*-

results = [{'telephoneNumber': ['99909-304'], 'info': ['799130'], 'wWWHomePage': ['www.dom.de'], 'physicalDeliveryOfficeName': ['C004'], 'name': ['Meier Karl'], 'co': ['Deutschland'], 'title': ['Sachbearbeiter'], 'facsimileTelephoneNumber': ['99909-77304'], 'company': ['PythonComp'], 'l': ['Tuxhausen'], 'st': ['Bayern'], 'department': ['EDV'], 'streetAddress': ['Bahnweg 2'], 'sn': ['Meier'], 'mail': ['Karl.Meier@PythonComp.de'], 'postalCode': ['84205'], 'displayName': ['Meier, Karl'], 'givenName': ['Karl'], 'initials': ['KM']}]

import sqlite3, os

if os.path.exists('mitarbeiter.db'):
    print 'Entferne die alte Datenbank.'
    os.remove('mitarbeiter.db')
else:
    print 'Datenbank nicht vorhanden.'

con = sqlite3.connect('mitarbeiter.db')
con.text_factory = str
cur = con.cursor()

cur.execute("""CREATE TABLE personen (
    nachname TEXT,
    vorname TEXT,
    personalnummer INTEGER,
    telefon INTEGER,
    fax INTEGER,
    abteilung TEXT,
    email TEXT)""")

# Dieser Abschnitt gefällt mir gar nicht. Das Auslesen von results muss besser gehen.
for line in results:
    if 'info' in line:
        # Variablen initialisieren, wenn mal ein Key im Dictionary fehlt
        nachname = vorname = personalnummer = telefon = fax = abteilung = email = ''
        if 'company' in line:
            firma = line['company'][0]
        if 'info' in line:
            personalnummer = line['info'][0]
        if 'department' in line:
            abteilung = line['department'][0]
        if 'givenName' in line:
            vorname = line['givenName'][0]
        if 'sn' in line:
            nachname = str(line['sn'][0])
        if 'streetAddress' in line:
            strasse = line['streetAddress'][0]
        if 'postalCode' in line:
            postleitzahl = line['postalCode'][0]
        if 'l' in line:
            ort = line['l'][0]
        if 'st' in line:
            bundesland = line['st'][0]
        if 'co' in line:
            land = line['co'][0]
        if 'mail' in line:
            email = line['mail'][0]
        if 'initials' in line:
            initialen = line['initials'][0]
        if 'physicalDeliveryOfficeName' in line:
            buero = line['physicalDeliveryOfficeName'][0]
        if 'telephoneNumber' in line:
            telefon = line['telephoneNumber'][0]
            telefon = telefon[-3:]
        if 'facsimileTelephoneNumber' in line:
            fax = line['facsimileTelephoneNumber'][0]
            fax = fax [-5:]
        if 'title' in line:
            position = line['title'][0]
        if 'wWWHomePage' in line:
            homepage = line['wWWHomePage'][0]
        if 'manager' in line:
            vorgesetzter = line['manager'][0]

        werte = (nachname, vorname, personalnummer, telefon, fax, abteilung, email)
        print werte
        sql = "INSERT INTO personen VALUES (?, ? ,? ,? ,? ,? ,?)"
        cur.execute(sql, werte)
        
con.commit()
con.close()

[BlackJack]

@peddy: `line` ist irgenwie ein schlechter Name für ein Wörterbuch.

Dann passiert abgesehen von den Telefonnummern in jedem ``if`` das selbe. Das verletzt das DRY-Prinzip („Don't Repeat Yourself”). Hier könnte man eine Schleife über die Schlüssel machen, die einen interessieren und `werte` als Liste erstellen ohne dass man alles noch mal auf Python-Ebene an Namen bindet.

Der Wert von `sql` ändert sich innerhalb der Schleife nicht, kann also vor die Schleife gezogen werden.

Ungetestet:

Code: Alles auswählen

    sql = 'INSERT INTO personen VALUES (?, ?, ?, ?, ?, ?, ?)'
    key2suffix_length = {
        facsimileTelephoneNumber: 5,
        telephoneNumber: 3,
    }
    for record in results:
        if 'info' in record:
            values = [
                record.get(key, [''])[0][-key2suffix_length.get(key, 0):])
                for key in [
                    'sn', 'givenName', 'info', 'telephoneNumber',
                    'facsimileTelephoneNumber', 'department', 'mail',
                ]
            ]
            print values
            cursor.execute(sql, values)

[Hyperion]

Also zuerst einmal fällt auf, dass die Datenstruktur irgend wie "ungünstig" erscheint; wenn man immer nur den *ersten Eintrag zu einem Schlüssel nimmt, dann kann man sich doch die inneren Listen sparen Aber vielleicht hängt das ja mit diesem "AD" zusammen - was auch immer das ist?

Du kennst doch die Schlüssel, die Dich interessieren - also merke Dir doch diese in einer Liste und erstelle aus dieser Schlüsselliste dann die Wertemenge für das Insert-Statement:

Code: Alles auswählen

keys = ("sn", "givenName", "info", ...)
for entry in result:
    values = [entry.get(key, [""])[0] for key in keys]
# values hält nun alle Werte

Edit: Ok, BlackJack war schneller und hat die robustere Lösung präsentiert

[BlackJack]

@Hyperion: AD = Active Directory. AFAIK liefert das diese Struktur.

[peddy]

Vielen Dank schon mal. Das Beispiel von BlackJack habe ich übernommen und musste nur zwei winzige Änderungen vornehmen, damit es lief.

Um ehrlich zu sein, war das Beispiel ziemlich scher für mich zu verstehen, da alles so komprimiert passiert. Für einen Novizen wie mich ist das zum Beispiel einfacher zu verstehen

Code: Alles auswählen

telefon = line['telephoneNumber'][0]
telefon = telefon[-3:]

als das.

Code: Alles auswählen

telefon = line['telephoneNumber'][0][-3:]

Das Beispiel von BlackJack war noch ein paar Nummern größer. Einzelne Fragmente habe ich zwar verstanden, aber das Zusammenspiel zu verstehen ist hart. Mit am meisten verwirrt hat mich die innerste for-Schleife (und das tut sie noch immer, etwas). Wie kann der Kern eine Auswirkung auf die Hülle haben? Wie dringt der Inhalt der Variable key nach außen und kann außerhalb der for-Schleife verwendet werden. Das verursacht Knoten in meinem Hirn.

Die get Function ist natürlich sehr hilfreich, da sie auch einen Wert liefern kann, wenn ein Key im Dictionary mal nicht vorhanden ist. Genau das Problem hatte ich nämlich.

Ich habe definitiv viel an diesem Beispiel gelernt und jetzt gehen ich noch mal über die for-Schleife nachdenken

Code: Alles auswählen

#!/usr/bin/env python
# -*- coding: UTF-8 -*-

results = [{'telephoneNumber': ['99909-304'], 'info': ['799130'], 'wWWHomePage': ['www.dom.de'], 'physicalDeliveryOfficeName': ['C004'], 'name': ['Meier Karl'], 'co': ['Deutschland'], 'title': ['Sachbearbeiter'], 'facsimileTelephoneNumber': ['99909-77304'], 'company': ['PythonComp'], 'l': ['Tuxhausen'], 'st': ['Bayern'], 'department': ['EDV'], 'streetAddress': ['Bahnweg 2'], 'sn': ['Meier'], 'mail': ['Karl.Meier@PythonComp.de'], 'postalCode': ['84205'], 'displayName': ['Meier, Karl'], 'givenName': ['Karl'], 'initials': ['KM']}]

import sqlite3, os

if os.path.exists('mitarbeiter.db'):
    print 'Entferne die alte Datenbank.'
    os.remove('mitarbeiter.db')
else:
    print 'Datenbank nicht vorhanden.'

con = sqlite3.connect('mitarbeiter.db')
con.text_factory = str
cur = con.cursor()

cur.execute("""CREATE TABLE personen (
    nachname TEXT,
    vorname TEXT,
    personalnummer INTEGER,
    telefon INTEGER,
    fax INTEGER,
    abteilung TEXT,
    email TEXT)""")

sql = 'INSERT INTO personen VALUES (?, ?, ?, ?, ?, ?, ?)'
key2suffix_length = {'facsimileTelephoneNumber': 5, 'telephoneNumber': 3,}

for record in results:
    if 'info' in record:
        values = [
            record.get(key, [''])[0][-key2suffix_length.get(key, 0):]
            for key in [
                'sn', 'givenName', 'info', 'telephoneNumber',
                'facsimileTelephoneNumber', 'department', 'mail',
            ]
        ]
        print values
        cur.execute(sql, values)
        
con.commit()
con.close()

[BlackJack]

@peddy: Das innere ist keine ``for``-Schleife sondern eine „list comprehension”. Damit kann man Listen erzeugen. Ganz grob:

Code: Alles auswählen

result = list()
for item in iterable:
    if test(item):
        result.append(func(item))

# <->

result = [func(item) for item in iterable if test(item)]

Der ``if``-Teil ist optional und statt `func()` und `item()` kann da natürlich jeder Python-Ausdruck stehen.

[Sirius3]

Hallo peddy,

ich persönlich bevorzuge ja die Variante von INSERT, bei der die Feldnamen mit angegeben werden.
Das vereinfacht Änderungen an der Datenbankstruktur. Einfach ad2sql anpassen, fertig.

Code: Alles auswählen

ad2sql = {
    'nachname': ('sn', str),
    'vorname': ('givenName', str),
    'personalnummer': ('info', int),
    'telefon': ('telephoneNumber', lambda val: int(val.split('-')[-1])),
    'fax': ('facsimileTelephoneNumber', lambda val: int(val.split('-')[-1])),
    'abteilung': ('department', str),
    'email': ('mail', str),
}

sql = 'INSERT INTO personen ({0}) VALUES ({1})'.format(
    ','.join("'%s'"%key for key in ad2sql.iterkeys()),
    ','.join("?"*len(ad2sql))
)

for record in results:
    if 'info' in record:
        values = [
            convert(record.get(key, [''])[0])
            for key, convert in ad2sql.itervalues()
        ]
        print values
        cur.execute(sql, values)

[peddy]

Ach so, values = [ ... ] ist eine „list comprehension”. Jetzt wird mir alles klar.

Danke.

[peddy]

Hallo peddy,

ich persönlich bevorzuge ja die Variante von INSERT, bei der die Feldnamen mit angegeben werden.
Das vereinfacht Änderungen an der Datenbankstruktur. Einfach ad2sql anpassen, fertig.

Ok, danke. Ich schaue mir das heute Abend genauer an. Jetzt muss ich leider weg.

[cofi]

Ach so, values = [ ... ] ist eine „list comprehension”. Jetzt wird mir alles klar.

Nein, eckige Klammern machen noch keine LC, sondern eine Liste. Mehr zu LC gibts hier: http://tutorial.pocoo.org/datastructure ... rehensions

[peddy]

Ach so, values = [ ... ] ist eine „list comprehension”. Jetzt wird mir alles klar.

Nein, eckige Klammern machen noch keine LC, sondern eine Liste. Mehr zu LC gibts hier: http://tutorial.pocoo.org/datastructure ... rehensions

Danke für den Hinweis. Das war mir in diesem Fall bewusst. Ich wollte schon auf das gesamte Konstrukt bezug nehmen, habe mich nur etwas zu knapp ausgedrückt.

[peddy]

@ Sirius3

Also deine Lösung hat mir noch mehr Kopfweh bereitet, aber ich denke jetzt bin ich endlich durchgestiegen.

Code: Alles auswählen

convert(record.get(key, [''])[0])

Die Variable convert ist ein Platzhalter für eine Funktion, z.B. str(), int(. Die dritte Möglichkeit verursacht aber Knoten in meinem Kopf. Ich verstehe zwar jetzt was eine Lambda-Funktion ist und was in diesem Fall passiert, aber so richtig einleuchtend ist es nicht.

record.get(key, [''])[0] liefert mir einen Wert, z.B eine Telefonnummer. Das wird dann an lamda var: übergeben, damit irgend wann int(304) raus kommt.

Nächste Denksportaufgabe:

Code: Alles auswählen

sql = 'INSERT INTO personen ({0}) VALUES ({1})'.format(
    ','.join("'%s'"%key for key in ad2sql.iterkeys()),
    ','.join("?"*len(ad2sql))
)

"'%s'"%key machst du wohl damit die Keys in Hochkomma stehen, z.B. 'nachname','vorname',... - oder? Mir wollte nicht so recht einleuchten warum das gemacht wird und daher habe ich probiert, ob es auch ohne geht.

Code: Alles auswählen

sql = 'INSERT INTO personen ({0}) VALUES ({1})'.format(
    ','.join(key for key in ad2sql.iterkeys()),
    ','.join("?"*len(ad2sql))
)

Beide Varianten funktionieren. Ist eine davon vorzuziehen, oder ist das egal?

[Sirius3]

Ein lambda ist nichts anderes als eine kurze Funktion, die nur aus einem Ausdruck besteht.
Hätte man auch so schreiben können:

Code: Alles auswählen

def telefonnummer_to_int(telefonnummer):
    return int(telefonnummer.split('-')[-1])

und in ad2sql bei Telefonnummer und Fax einfach telefonnummer_to_int benutzen können.
Da bei beiden die selbe Funktion verwendet wird, wäre diese Variante wohl sogar besser.

Für Feldnamen kann man bei sqlite leider keine ?-Platzhalter benutzen, daher meine Vorsicht,
alles nochmal in '' zu packen. Ist natürlich unnötig, wenn man weiß, dass ad2sql.keys nur
gutartige Strings enthält. Dann ist auch die for-Schleife unnötig:

Code: Alles auswählen

sql = 'INSERT INTO personen ({0}) VALUES ({1})'.format(
    ','.join(ad2sql.iterkeys()),
    ','.join("?"*len(ad2sql))
)