Adressdatenbank

[gerold]

Irgendwie bekomme ich Endlosschleifen

Hallo CrackPod!

Du musst deinen Code **debuggen**. Also, an strategisch guten Stellen Haltepunkte setzen und nachverfolgen in welcher Endlosschleife dein Programm stehen bleibt. Schritt für Schritt. Dann musst du deine Schleifen-Abbruchbedingung prüfen. Z.B. den Wert von Variablen abfragen. Daraus kannst du dann normalerweise schließen, warum das Programm in der Schleife stehen bleibt.

Ich verwende zum Debuggen WingIDE. Du kannst aber genauso gut mit dem PDB http://docs.python.org/dev/lib/module-pdb.html oder einer anderen IDE zum Debuggen arbeiten.

http://www.digitalpeers.com/pythondebugger/
http://bashdb.sourceforge.net/pydb/
http://www.thescripts.com/forum/thread22783.html

lg
Gerold

[BlackJack]

Das Programm ist ziemlich gross um mal eben drüber zu schauen. Und es lässt sich nicht starten weil die `./daten/settings.ini` fehlt. Wobei mir da im Quelltext erstmal aufgefallen ist, das Du Dateien immer mit '+' Modus öffnest, auch wenn Du gar nichts schreiben willst.

Den eigenen Parser für Konfigurationsdateien hättest Du Dir sparen können, so etwas bekommt man schon in der Standardbibliothek: `ConfigParser`.

Nachdem ich die Konfigurationsdatei erzeugt habe und mit den entsprechenden Werten gefüllt habe, scheitere ich an folgender Ausnahme:

Code: Alles auswählen

Traceback (most recent call last):
  File "address.py", line 508, in ?
    main()
  File "address.py", line 489, in main
    adr = Adress(cfgData.getDBPath())
  File "address.py", line 26, in __init__
    self.con.row_factory = sqlite3.Row
AttributeError: 'module' object has no attribute 'Row'

`Row` gibt's bei mir in `pysqlite2.dbapi2` nicht.

Also schnell(?) die Python 2.5b2 übersetzt, da geht's dann. Und eine Endlosschleife bei `vorname` und `Max` kann ich nicht nachvollziehen.

Stilkritik: Es gibt einige Zeilen, die länger als 80 Zeichen lang sind. Ein paar Namen sind nicht PEP008 "tauglich" und nach Kommata und vor und nach Operatoren ist ein Leerzeichen ganz nett. Wo es eher nicht hingehört ist zwischen Name und Parameterliste von Funktionsdefinitionen und das ausrichten von Gleichheitszeichen ist auch keine gute Angewohnheit. Spätestens wenn man mit einer Versionsverwaltung wie CVS oder Subversion arbeitet bringt das Probleme mit sich. Wenn man nämlich Änderungen an einem Namen vornimmt und dann alle Gleichheitszeichen neu ausrichtet, kann man im Diff nicht mehr so schnell sehen, was eigentlich die Änderung war und was nur Kosmetik.

Ausnahmebehandlungen sollten immer eine konkrete Ausnahme behandeln. Beim importieren wäre das also ein `ImportError`.

Auf `__del__()` sollte man sich nicht verlassen. Eine Methode, welche die Verbindung zur Datenbank schliesst und die vom Benutzer aufgerufen werden muss, ist sicherer und deterministischer.

Die Liste mit den ganzen Attributnamen einer Adresse taucht viel zu oft im Programm auf. Alleine in `Adress.add()` wird jeder Name 5 mal wiederholt. Jede Wiederholung birgt die Gefahr von Fipptehlern und macht Änderungen aufwändiger.

Bei `Adress.get()` setzt Du Dich der Gefahr einer SQL-Injection aus. Jemand kann "böse" Suchbegriffe eingeben um beliebiges SQL ausführen zu lassen. Ausserdem ist das Konstrukt mit der ``while`` Schleife eigenartig. Warum speicherst Du Daten auf die über einen fortlaufenden Index zugegriffen wird in einem Dictionary? Eine Liste wäre die "natürlichere" Wahl dafür. Man kann über Cursor-Objekte iterieren, damit lässt sich der wiederholte Aufruf von `fetchone()` sparen. Eine Liste mit Dictionaries für die einzelnen Rows kann man zum Beispiel so bekommen:

Code: Alles auswählen

        def row_to_dict(row):
            return dict((name, row[name]) for name
                        in ('id', 'vorname', 'zuname', 'nachname', 'anrede',
                            'adresse', 'plz', 'ort', 'land', 'privatnummer',
                            'privatfaxnummer', 'mobilnummer', 'bueronummer',
                            'buerofaxnummer', 'email'))
        
        adresses = map(row_to_dict, self.cur)

Warum wird eigentlich `None` zurückgegeben, wenn kein Ergebnis gefunden wurde?

In `Adress.delete()` beim Löschen wäre es sinnvoller gleich über `adresses.values()` zu iterieren statt über die Schlüssel und dann in der Schleife den Wert zu holen. Das wäre ausserdem ein guter Anwendungsfall für `executemany()`.

Das `Adress.getall()` copy'n'paste Quelltext ist, fiel Dir laut Kommentar ja schon selbst auf. `getall()` ist nur ein Sonderfall von der Suche, als solchen sollte man ihn auch behandeln können.

Bei `Adress.out()` iterierst Du wieder über die Schlüssel statt über die Einträge. Diesmal resultiert daraus ganz viel Schreibarbeit weil Du etliche male ``adresses[key]`` tippen musst statt einfach gleich über die Adressen selbst zu iterieren.

Das Telefonbuch zu löschen, indem man die Datei auf die harte Tour "leert" ist ein wenig unelegant. Besser wäre es wohl die Tabelle per SQL zu leeren. Dann braucht man die nicht wieder anzulegen um neue Daten einzutragen.

Der Konfigurationsparser ist ziemlich fragil. In `TCfgP.__init__()` wird wieder eine komische ``while``-Schleife benutzt die man einfach durch iterieren über das Objekt ersetzen kann. In diesem Fall also einfach ``for line in f:``.

`TCfgP.getDBPath()` sollte auf jeden Fall `os.path.join()` für den Dateinamen benutzen. Ich musste meine Datenbank erstmal suchen.

Ein Entwurfsproblem ist das Mischen von Benutzeroberfläche und Logik. Das solltest Du besser trennen.

Sooo, ich hoffe das war erstmal genug.

[CrackPod]

Danke BlackJack. Ich werde mir mal deine Verbesserungsvorschläge zu Herzen nehmen und nach und nach alles ändern.
Ich hab mit dem SQL Code angefangen:

Code: Alles auswählen

#Aus dem Konstruktor:
        #Tupel mit den Spalten der Datenbank
        self.dbCols = ('id', 'vorname', 'zuname', 'nachname', 'anrede', 'adresse', 'plz', 'ort', 'land',
                       'privatnummer', 'privatfaxnummer', 'mobilnummer', 'bueronummer', 'buerofaxnummer', 'email')

#def add():
        sqlCols = ''
        sqlPlazeHolders = ''

        for cols in self.dbCols:
            sqlCols += cols + ','
            sqlPlazeHolders += ':' + cols + ','
        sqlCols = sqlCols[:-1]
        sqlPlazeHolders = sqlPlazeHolders[:-1]

        print sqlCols
        print sqlPlazeHolders

        sql = 'INSERT INTO addresses (%s) VALUES (%s)' % (sqlCols, sqlPlazeHolders)

        print sql

        #Führt den SQL-Befehl aus und ersetzt die Platzhalter (Per Dictionary)
        self.cur.execute(sql,{'vorname':vorname, 'zuname':zuname,
            'nachname':nachname, 'anrede':anrede, 'adresse':adresse, 'plz':plz,
            'ort':ort, 'land':land, 'privatnummer':privatnummer, 'privatfaxnummer':privatfaxnummer,
            'mobilnummer':mobilnummer, 'bueronummer':bueronummer, 'buerofaxnummer':buerofaxnummer, 'email':email})
        self.con.commit()

Spalten:
id,vorname,zuname,nachname,anrede,adresse,plz,ort,land,
privatnummer,privatfaxnummer,mobilnummer,
bueronummer,buerofaxnummer,email

Platzhalter
:id,:vorname,:zuname,:nachname,:anrede,:adresse,
:plz,:ort,:land,:privatnummer,:privatfaxnummer,:mobilnummer,
:bueronummer,:buerofaxnummer,:email

SQL:
INSERT INTO addresses (id,vorname,zuname,nachname,anrede,
adresse,plz,ort,land,privatnummer,privatfaxnummer,
mobilnummer,bueronummer,buerofaxnummer,email)
VALUES (:id,:vorname,:zuname,:nachname,:anrede,
:adresse,:plz,:ort,:land,:privatnummer,:privatfaxnummer,
:mobilnummer,:bueronummer,:buerofaxnummer,:email)

Traceback (most recent call last):
File "/home/gottesworkstation/Python/Adressenverwaltung.py", line 499, in -toplevel-
main()
File "/home/gottesworkstation/Python/Adressenverwaltung.py", line 477, in main
adr.add('Tobias','Helmut','Hoebel','Herr','Togostr 19','81827','Muenchen','Deutschland','08943571732','','01793248225','','','tobsi-h@gmx.de')
File "/home/gottesworkstation/Python/Adressenverwaltung.py", line 116, in add
self.cur.execute(sql,{'vorname':vorname, 'zuname':zuname, 'nachname':nachname, 'anrede':anrede, 'adresse':adresse, 'plz':plz,
ProgrammingError: You did not supply a value for binding 1.

Versteh den Fehler nicht. Das execute hat davor prima funktioniert, der SQL Code stimmt egtl auch. Erkennt ihr den Fehler?

Zu den SQLInjections: Ich weiß nicht, wie ich die umgehen soll, bei search... Andererseits hab ich mir gedacht, dass es egal ist, da es ja eh nur ein Adressbuch ist und man da nich so viel vertrauliche Daten draus gewinnen kann

Greetz

EDIT:
Ich habe den Fehler gefunden. Ich hatte keinen Wert für die Spalte ID angegeben.

[BlackJack]

Code: Alles auswählen

#Aus dem Konstruktor:
        #Tupel mit den Spalten der Datenbank
        self.dbCols = ('id', 'vorname', 'zuname', 'nachname', 'anrede', 'adresse', 'plz', 'ort', 'land',
                       'privatnummer', 'privatfaxnummer', 'mobilnummer', 'bueronummer', 'buerofaxnummer', 'email')

#def add():
        sqlCols = ''
        sqlPlazeHolders = ''

        for cols in self.dbCols:
            sqlCols += cols + ','
            sqlPlazeHolders += ':' + cols + ','
        sqlCols = sqlCols[:-1]
        sqlPlazeHolders = sqlPlazeHolders[:-1]

Die sieben letzten Zeilen lassen sich auch in zwei ausdrücken:

Code: Alles auswählen

        sql_cols = ','.join(self.DB_COLS)
        sql_place_holders = ','.join(':' + name for name in self.DB_COLS)

[CrackPod]

Hm, stimmt. An join hab ich gar nich gedacht...
Gibts dafür auch ne Kurzversion:

Code: Alles auswählen

sqlCols = ''
        sqlPlazeHolders = ''
        colException = ('id',)
        for col in self.dbCols:
            if col not in colException:
                sqlCols += col + ', '
                sqlPlazeHolders += ':' + col + ', '
        sqlCols = sqlCols[:-2]
        sqlPlazeHolders = sqlPlazeHolders[:-2]

        sql = 'INSERT INTO addresses (%s) VALUES (%s)' % (sqlCols, sqlPlazeHolders)
        print sql

Ich will nur Spalten haben, die ich auch brauch, weswegen ich colException brauch. Da frag ich ab, ob der Wert von dbCols auch in colException steht - wenn ja, wird er nich eingetragen.

Code: Alles auswählen

        def row_to_dict(row): 
             return dict((name, row[name]) for name 
                         in ('id', 'vorname', 'zuname', 'nachname', 'anrede', 
                             'adresse', 'plz', 'ort', 'land', 'privatnummer', 
                             'privatfaxnummer', 'mobilnummer', 'bueronummer', 
                             'buerofaxnummer', 'email')) 
         
         adresses = map(row_to_dict, self.cur)

Kannst du mir bitte die map() Funktion erklären? Irgendwie versteh ich die nich. Aus der Doku werd ich nich schlau.
Am besten gleich die ganze Funktion

Greetz

[BlackJack]

Hm, stimmt. An join hab ich gar nich gedacht...
Gibts dafür auch ne Kurzversion:

Code: Alles auswählen

sqlCols = ''
        sqlPlazeHolders = ''
        colException = ('id',)
        for col in self.dbCols:
            if col not in colException:
                sqlCols += col + ', '
                sqlPlazeHolders += ':' + col + ', '
        sqlCols = sqlCols[:-2]
        sqlPlazeHolders = sqlPlazeHolders[:-2]

        sql = 'INSERT INTO addresses (%s) VALUES (%s)' % (sqlCols, sqlPlazeHolders)
        print sql

Ich will nur Spalten haben, die ich auch brauch, weswegen ich colException brauch. Da frag ich ab, ob der Wert von dbCols auch in colException steht - wenn ja, wird er nich eingetragen.

Ermittle vorher einfach die Spalten, die Du brauchst und benutz dann `join()` wie vorher.

Code: Alles auswählen

columns = set(DB_COLS) - set(col_exception)

Durch die `set`\s sind die Namen dann nicht mehr zwingend in der gleichen Reihenfolge wie in den Listen, aber das macht in diesem Fall ja nichts aus.

Code: Alles auswählen

        def row_to_dict(row): 
             return dict((name, row[name]) for name 
                         in ('id', 'vorname', 'zuname', 'nachname', 'anrede', 
                             'adresse', 'plz', 'ort', 'land', 'privatnummer', 
                             'privatfaxnummer', 'mobilnummer', 'bueronummer', 
                             'buerofaxnummer', 'email')) 
         
         adresses = map(row_to_dict, self.cur)

Kannst du mir bitte die map() Funktion erklären? Irgendwie versteh ich die nich. Aus der Doku werd ich nich schlau.
Am besten gleich die ganze Funktion

Die Funktion bekommt eine Funktion und ein oder mehrere iterierbare Objekte und wendet dann die übergebene Funktion auf jedes Element an und speichert das Ergebnis in einer Liste.

Code: Alles auswählen

In [4]: def double(n):
   ...:     return n * 2
   ...:

In [5]: map(double, [1, 2, 23, 42])
Out[5]: [2, 4, 46, 84]

Wenn man die Funktion `map()` selbst implementieren wollte, würde sie ungefähr so aussehen:

Code: Alles auswählen

def my_map(func, iterable):
    result = list()
    for item in iterable:
        result.append(func(item))
    return result

[CrackPod]

ok, aber self.cur enthält doch keine Daten oder doch?!
Und was macht set genau?

Return a set[...]

Was ist ein "set"?

The elements must be immutable.

Was bedeutet, dass es "immutable" sein muss und was is das egtl?

[...]inner sets should be frozenset objects.

Was ist ein "frozenset"? (Wär wahrscheinlich schnell beantwortet, wenn ich wüsste was ein set wär )

Das sind warscheinlich mal wieder ziemliche newbie Fragen

[Leonidas]

ok, aber self.cur enthält doch keine Daten oder doch?!
Und was macht set genau?

Return a set[...]

Was ist ein "set"?

Set ist das englische Wort für Menge. Das Set in Python ist einfach eine Art Liste, die nur verschiedene Elemente enthält (also keine Elemente doppelt), die im Gegensatz zu einer Liste auch keine bestimmte Reihenfolge haben. Wie eine Menge eben.

The elements must be immutable.

Was bedeutet, dass es "immutable" sein muss und was is das egtl?

Immutable das heißt so viel wie unveränderbar - das die Objekte, nachdem sie erstellt worden sind, nicht mehr geändert werden können. Strings und Ints sind Beispiele für immutable Datentypen, mutable Datentypen sind zum Beispiel Listen.

[...]inner sets should be frozenset objects.

Was ist ein "frozenset"? (Wär wahrscheinlich schnell beantwortet, wenn ich wüsste was ein set wär )

Frozenset, auch bekannt als ImmutableSet ist einfach die immutable Version des Sets.

[BlackJack]

ok, aber self.cur enthält doch keine Daten oder doch?!

Cursor Objekte sind "iterierbar" das heisst man kann von ihnen einen Iterator über die Ergebniszeilen bekommen. Also kann man so einen Cursor zum Beispiel so benutzen:

Code: Alles auswählen

cursor.execute('SELECT * FROM a_table')
for row in cursor:
    print row

[CrackPod]

Achso... Ich habs nur mit nem Print versucht und hab mir gedacht, dann müsste ja sowas wie ein Tupel oder eine Liste rauskommen(isses aber nich )
Und wie sieht das mit set() aus? was macht das? Ich komm mit englischen Dokumentationen von Programmiersprachen überhaupt nich klar - zu kompliziert

[Leonidas]

Und wie sieht das mit set() aus? was macht das? Ich komm mit englischen Dokumentationen von Programmiersprachen überhaupt nich klar - zu kompliziert

Hab ich doch geschrieben - lies doch mal mein letztes Posting.

[CrackPod]

Und wie sieht das mit set() aus? was macht das? Ich komm mit englischen Dokumentationen von Programmiersprachen überhaupt nich klar - zu kompliziert

Hab ich doch geschrieben - lies doch mal mein letztes Posting.

Tut mir leid, den hab ich übersehen.

Danke,
Greetz

[CrackPod]

Ich habe jetzt mal deinen Code hergenommen (und soweit eigentlich auch verstanden). Nur, was ich noch nich versteh is, wie ich in der Liste der Datenbankspalten suchen kann. Das mit der Forschleifen geht nicht:

Code: Alles auswählen

#------------------------------------------------------------
    def match(self, row):
        return dict((name, row[name]) for name in (self.dbCols))

#------------------------------------------------------------
    def get(self, args, getall = False):
        '''Suche nach Kontakten

        :param args: Dictionary für die Suche. Keys sind die Spalten, in denen gesucht wird; Values die Werte für die Spalten.
        '''

        #Erstellt den SQL-Befehl und erstellt die WHERE-Klausel wobei - falls mehrere Argumente angegeben worden sind - diese mit einem AND verknüpft werden
        sql = '''SELECT %s FROM addresses''' % ', '.join(self.dbCols)

        self.cur.execute(sql)

        adresses = map(self.match, self.cur)

        return adresses

Code: Alles auswählen

    foo = adr.get({'vorname':'Max','nachname':'Musterman'})
    for col in foo:
        for key, val in col.items():
            print key,'==>',val

---------------
Datensatz nummer: 1
mobilnummer ==> 01701234586
land ==> Deutschland
zuname ==> Maximilian
ort ==> Musterstadt
anrede ==> Herr
adresse ==> Musterstr 12
plz ==> 1234865
bueronummer ==>
id ==> 1
email ==> max.musterman@muster.de
privatfaxnummer ==>
nachname ==> Musterman
buerofaxnummer ==>
privatnummer ==> 004912326548
vorname ==> Max
---------------
Datensatz nummer: 2
mobilnummer ==> 01701234586
land ==> Deutschland
zuname ==> Maximilian
ort ==> Musterstadt
anrede ==> Herr
adresse ==> Musterstr 12
plz ==> 1234865
bueronummer ==>
id ==> 2
email ==> max.musterman@muster.de
privatfaxnummer ==>
nachname ==> Musterman
buerofaxnummer ==>
privatnummer ==> 004912326548
vorname ==> Max II.
---------------
Datensatz nummer: 3
mobilnummer ==> 01701234586
land ==> Kino
zuname ==> Umzumzumz
ort ==> Polar
anrede ==> aus der
adresse ==> Eisstr 12
plz ==> 1234865
bueronummer ==>
id ==> 3
email ==> murmel.aus_dem-eis@kalterPolar.de
privatfaxnummer ==>
nachname ==> Murmel
buerofaxnummer ==>
privatnummer ==> 004912326548
vorname ==> schubdidub

Fazit: Nunja, die Suche geht nicht Oder hab ich beim übernehmen nur einen Fehler gemacht? Allerdings kann ich mir nicht so ganz vorstellen, dass man einfach so eine Liste übergeben kann, in der dann gesucht wird, ich mein, die Liste der Spalten hat ja keinen Suchwert?!(wenn ihr jetzt versteht, was ich mein)
Greetz

[BlackJack]

Ich verstehe die Frage nicht ganz. Du suchst ja auch nirgends, jedenfalls nicht in den Quelltextschnipseln, die Du eben angegeben hast.

Da steht zwar ein Kommentar bei der SQL Zeichenkette über eine ``WHERE`` Klausel, aber die steht da nirgends!?

[CrackPod]

Also ich hab das ganze jetz nochma genau angeschaut und wieder ein wenig mehr verstanden
So sieht der ganze Spaß bis jetz aus:

Code: Alles auswählen

#------------------------------------------------------------
    def match(self, row):

        data = dict((name, row[name]) for name in (self.dbCols))
        if 'Max' in data.values():
            return data

#------------------------------------------------------------
    def get(self, args, getall = False):
        '''Suche nach Kontakten

        :param args: Dictionary für die Suche. Keys sind die Spalten, in denen gesucht wird; Values die Werte für die Spalten.
        '''

        #Erstellt den SQL-Befehl und erstellt die WHERE-Klausel wobei - falls mehrere Argumente angegeben worden sind - diese mit einem AND verknüpft werden
        sql = '''SELECT %s FROM addresses''' % ', '.join(self.dbCols)

        self.cur.execute(sql)

        adresses = map(self.match, self.cur)

        self.out(adresses)

Aufruf:

Code: Alles auswählen

    adr.add('Max','Maximilian','Musterman','Herr','Musterstr 12',
'1234865','Musterstadt','Deutschland','004912326548','',
'01701234586','','','max.musterman@muster.de')
    adr.add('Max II.','Maximilian','Musterman','Herr','Musterstr 12',
'1234865','Musterstadt','Deutschland','004912326548','',
'01701234586','','','max.musterman@muster.de')
    adr.add('schubdidub','Umzumzumz','Murmel','aus der','Eisstr 12',
'1234865','Polar','Kino','004912326548','','01701234586','','',
'murmel.aus_dem-eis@kalterPolar.de')
    adr.get({'nachname':'Musterman',})

Funktioniert auch eigentlich Super... Nur: Wenn ich map benutze: Wie kann ich bei match() noch einen Parameter übergeben?! Also den, damit ich einen String für die Suche angeben kann?
Stelle:

Code: Alles auswählen

#------------------------------------------------------------
    def match(self, row):

        data = dict((name, row[name]) for name in (self.dbCols))
        if 'Max' in data.values(): # 'Max' <-- muss ersetzt werden - Aber wie?
            return data

Ich weiß nicht, wie ich das machen kann...

Fazit: Z.Z. ist es kackegal, ob ich bei adr.get() daten übergebe oder nicht -.- Es wird eh nur der Datensatz von Max rausgesucht :'(

[BlackJack]

Der Name `match` ist sehr ungünstig. Die Funktion ist nur dazu da um aus einem DictCursor Ergebnis ein echtes `dict` zu machen. Wenn die Bedingung ('Max') nicht zutrifft, dann gibt die Funktion `None` zurück.

Warum bastelst Du die Abfrage nicht schon in die SQL-Anweisung? Dann kümmert sich schon die Datenbank darum.

[CrackPod]

also doch mit einer WHERE Klausel?!
Von der hast du mir abgeraten; wegen SQL Injections - kann ich auch verstehen, nur wusst ich nich, wie ich die umgehen soll.
Im selben Posting haste dann diese row_to_dict Funktion gepostet, deswegen hab ich mir gedacht: Fein, dann hol ich alles, übergeb das der map und die sucht mir dann raus, welche Datensätze zutreffen...
Mittlerweile is mir aufgefallen, warum du mir die Funktion gezeigt hast ^^
Damit die While Schleifen wegkommen - ich habs nur falsch interpretiert
Ja gut, aber wie soll ich das dann machen, damit ich den Injections aus dem Weg geh?

[BlackJack]

Einfach das einsetzen der Werte dem Datenbankmodul überlassen. Schau Dir die Doku zur `execute()` Methode und dem zweiten Parameter an.

[CrackPod]

Einfach das einsetzen der Werte dem Datenbankmodul überlassen. Schau Dir die Doku zur `execute()` Methode und dem zweiten Parameter an.

Kannst du mir pls den Link zur Doku geben? Ich find die irgendwie nicht

Greetz

[gerold]

Link zur Doku

Hi CrackPod!

http://docs.python.org/dev/lib/sqlite3- ... jects.html
http://www.python.org/dev/peps/pep-0249/

lg
Gerold