Adressbuch, erster Versuch

Code-Stücke können hier veröffentlicht werden.
Graf Wasserrutsche
User
Beiträge: 37
Registriert: Donnerstag 17. Juli 2008, 06:59
Wohnort: Köln
Kontaktdaten:

Adressbuch, erster Versuch

Beitragvon Graf Wasserrutsche » Mittwoch 10. September 2008, 12:22

Guten Tag zusammen!

Ich habe mich jetzt nach der ersten Umsetzung der HangMan Projektidee von [wiki]Projektideen[/wiki] an dem Adressbuch versucht. Zunächst nur mit Pickle, die Umsetzung für SQL wird dann noch folgen, sollte der Code gut genug sein.

Könntet Ihr euch den Code mal anschauen und mir sagen, was alles verbesserungswürdig ist, oder was anders geregelt werden könnte? Ich bin mir nicht sicher, ob die Aufteilung der Funktionen und das verwenden der Klasse so 100% in Ordnung ist, außerdem wüsste ich nicht wie ich das immer wiederkehrende Menü ohne eine While-Schleife erledige.

Vielen Dank schonmal!

Code: Alles auswählen

import pickle
from sys import exit

class Person(object):
    def __init__(self):
        self.vorname = 'Max'
        self.name = 'Mustermann'
        self.geburtsort = 'Musterhausen'
        self.strasse = 'Musterstr.'
        self.hausnummer = '1'
        self.plz = '12345'
        self.stadt = 'Musterhausen'
   
    def __del__(self):
        print 'Person geloescht!'
       
def person_add():
    person = Person()
    person.vorname = raw_input('Vorname: ')
    person.name = raw_input('Nachname: ')
    person.geburtsort = raw_input('Geburtsort: ')
    person.strasse = raw_input('Strasse: ')
    person.hausnummer = int(raw_input('Hausnummer: '))
    person.plz = int(raw_input('PLZ: '))
    person.stadt = raw_input('Stadt: ')
    return person

def dump_dict(personen):
    file = open('personen/daten.pk1', 'wb')
    pickle.dump(personen, file)
    file.close()

def load_dict():
    file = open('personen/daten.pk1', 'rb')
    personen = pickle.load(file)
    file.close()
    return personen
       
def menue():
    print '1) Personen listen'
    print '2) Person hinzufuegen'
    print '3) Person loeschen'
    print '4) Programm beenden\n'
    i = int(raw_input('Waehlen Sie eine Option: '))
    return i

personen = load_dict()

if personen == '':
    print 'Personenliste ist leer! Bitte eine Person hinzufuegen'
    personen = person_add()
    personen = {personen.name:person}
    dump_dict(personen)

weiter = 'ja'
while weiter == 'ja':
    i = menue()
    if i == 1:
        x = 0
        for i in personen:
            x+=1
            print str(x) + ') ' + i
        beenden = 'nein'
        while beenden != 'ja':
            i = raw_input('\nWelche Person moechten Sie anzeigen?\n')
            person = personen[i]
            print 'Vorname: ' + person.vorname
            print 'Nachname: ' + person.name
            print 'Gebursort: ' + person.geburtsort
            print 'Strasse: ' + person.strasse
            print 'Hausnummer: ' + str(person.hausnummer)
            print 'PLZ: ' + str(person.plz)
            print 'Stadt: ' + person.stadt
            i = raw_input('\n1) Menue \n2) Weitere Person suchen')
            if i == '2':
                beenden = 'nein'
            elif i == '1':
                beenden = 'ja'
                weiter = 'ja'       
    elif i == 2:
        person = person_add()
        personen[person.name] = person
        dump_dict(personen)
    elif i == 3:
        x = 0
        for i in personen:
            x+=1
            print str(x) + ') ' + i
        i = raw_input('\nWelche Person moechten Sie loeschen?\n')
        del personen[i]
    elif i == 4:
        exit()
[/url]
BlackJack

Beitragvon BlackJack » Mittwoch 10. September 2008, 13:36

Der Max Mustermann hat in der `Person.__init__()` nichts zu suchen. Die ganzen Angaben sollten als Argumente übergeben werden. In `person_add()` können sie dann übergeben werden statt sie von aussen zu setzen. Der Name der Funktion ist etwas unpassend weil da nur eine Person erstellt, aber nirgends hinzugefügt wird.

`__del__()` sollte man nicht verwenden, schon gar nicht für so eine banala Ausgabe. Ob und wann die Methode aufgerufen wird, ist von der Sprachspezifikation nicht garantiert.

Postleitzahlen und Hausnummern sind vom Datentyp her keine Zahlen! Es gibt Postleitzahlen, die mit 0 anfangen, wobei die führende 0 bei `int()` dann einfach weg fällt und andere Länder die auch Buchstaben in "Postleitzahlen" haben. Es gibt Hausnummern wie '12b' oder bei grösseren Behörden und Firmen so etwas wie '40-42'.

Den Dateinamen könnte man als Konstante aus den Funktionen `dump_dict()` und `load_dict()` heraus ziehen.

Der Code auf Modulebene sollte in einer Funktion verschwinden.

Die einzelnen Funktionen aus dem Menü würde ich in eigene Funktionen stecken.

`weiter` und `beenden` sind IMHO verwirrend. `weiter` wird überhaupt nicht wirklich benutzt, das ist immer an 'ja' gebunden. "Endlosschleifen" mit ``while True:`` die ggf. mit ``break`` verlassen werden, sparen da je eine Variable.

Das die Personenliste leer ist, rechtfertigt keinen Sonderfall vor der Schleife. Zumal ich nicht sehe wann `personen` jemals die leere Zeichenkette sein soll, wenn das Programm immer Dictionaries in die Datei "pickled".

Während beim Hinzufügen einer Person die Datenbank gespeichert wird, passiert das nach dem Löschen nicht.

Fehler werden überhaupt nicht behandelt.

PS: Familien kann man auch nicht eingeben, immer nur eine Person pro Nachname.
Graf Wasserrutsche
User
Beiträge: 37
Registriert: Donnerstag 17. Juli 2008, 06:59
Wohnort: Köln
Kontaktdaten:

Beitragvon Graf Wasserrutsche » Dienstag 7. Oktober 2008, 11:57

Hallo zusammen!

So, habe die Änderungsvorschläge berücksichtigt und die Fehler ausgebessert, nun stehe ich allerdings vor dem Problem, welches BlackJack angesprochen hat.
Ich kann nicht mehrere Einträge von Personen machen, welche den gleichen Nachnamen besitzen.

Dazu habe ich mir folgendes gedacht:

Code: Alles auswählen

    for x in personen:
        if x == person.name:
            person.name = person.name + '_neu'
            print person.name


Hier prüfe ich, ob der Name schon mal im Dictionary vorhanden ist und wenn ja, dann wird an diesen ein `_neu` (neu steht hier für eine Zahl) angehangen. Dieser wird dann dem Dictionary hinzugefügt und gepickled.
Jetzt stehe ich vor dem Problem das zum einen immer nur der erste Name (ohne den Anhang) geprüft wird. Hierzu habe ich mir gedacht, dass ich es so machen könnte, dass ich erst prüfe ob überhaupt eine Zahl am Ende steht und wenn ja, dass ich diese Zahl in einer anderen Variable um eins erhöhe und dann hinzufüge.
So, nun stehe ich auf dem Schlauch und weiss nicht wirklich weiter. Wenn ich mit dem jetzigen Auslesen weiterarbeite, wie steuere ich die einzelnen Einträge dann an? Kann ich nach teilen in den Namen suchen? Also z.B. nach müller in Müller_1 / Müller_2 etc.? Besitzen die Einträgein Dicionaries auch extra Zahlenwerte die als Index fungieren, damit ich diese eher ansprechen kann? Ich hoffe ich habe mich nicht allzu dumm ausgedrückt, werde das noch einmal anders formulieren, falls nötig.

Oder die allgemeine Frage, befinde ich mich auf dem richtigen Weg? Dann versuche ich mehr an dieser Art und Weise weiterzuarbeiten. Und ich habe das Gefühl, dass sich mein Code immer so unkontrolliert aufbläht, aber ich wüsste auch nicht wie ich das anders regeln könnte. Die Fehlerbehandlung werde ich in Angriff nehmen, wenn ich dieses Problem beseitigt habe.

Viele Dank schon mal für die Mühe!

Anbei ist noch mal der restliche unvollständige Code:

Code: Alles auswählen

import pickle

from sys import exit

class Person(object):
    def __init__(self):
        pass
       
def person_new():
    person = Person()
    person.vorname = raw_input('Vorname: ')
    person.name = raw_input('Nachname: ')
    person.geburtsort = raw_input('Geburtsort: ')
    person.strasse = raw_input('Strasse: ')
    person.hausnummer = raw_input('Hausnummer: ')
    person.plz = raw_input('PLZ: ')
    person.stadt = raw_input('Stadt: ')
    return person

def person_add(person):
    file = open('personen/daten.pk1', 'rb')
    personen = pickle.load(file)
   
    for x in personen:
        if x == person.name:
            person.name = person.name + '_neu'
            print person.name
   
    file.close()

def person_show():
    x = 0
    for i in personen:
        x+=1
        print str(x) + ') ' + i

def dump_dict():
    file = open('personen/daten.pk1', 'wb')
    pickle.dump(personen, file)
    file.close()

def load_dict():
    file = open('personen/daten.pk1', 'rb')
    personen = pickle.load(file)
    file.close()
    return personen
       
def menue_haupt():
    print '1) Personen listen'
    print '2) Person hinzufuegen'
    print '3) Person loeschen'
    print '4) Programm beenden'
    i = int(raw_input('\nWaehlen Sie eine Option: '));print ''
    return i

def menue_list():
    i = raw_input('\nWelche Person?\n')
    person = personen[i]
    print 'Vorname: ' + person.vorname
    print 'Nachname: ' + person.name
    print 'Gebursort: ' + person.geburtsort
    print 'Strasse: ' + person.strasse
    print 'Hausnummer: ' + str(person.hausnummer)
    print 'PLZ: ' + str(person.plz)
    print 'Stadt: ' + person.stadt

def menue_delete():
    i = raw_input('\nWelche Person moechten Sie loeschen?\n')
    del personen[i]
    dump_dict()   

## Wieso kann `break` nicht ausgelagert werden?

def main():
    while True:
        i = menue_haupt()
        if i == 1:           
            while True:
                person_show()
                menue_list()
                i = int(raw_input('\n1) Menue\n2) Nochmal\n'))
                if i == 1:
                    break
        elif i == 2:
            person = person_new()
            person_add(person)
            i = int(raw_input('\n1) Menue\n2) Nochmal\n'))
            if i == 1:
                break
        elif i == 3:
            while True:
                person_show()
                menue_delete()
                i = int(raw_input('\n1) Menue\n2) Nochmal\n'))
                if i == 1:
                    break
        elif i == 4:
            exit()
   
personen = load_dict()         
main()
BlackJack

Beitragvon BlackJack » Dienstag 7. Oktober 2008, 12:40

Das ist auf jeden Fall der falsche Weg. Wenn man ständig Zahlen aus Zeichenketten raussuchen und hin- und her wandeln muss, dann macht man etwas falsch.

Wenn Du ein Dictionary von Nachnamen auf Adresseinträge haben willst, und es mehrere Einträge für einen Nachnamen geben kann, sollte man halt auch genau das tun: Als Wert mehrere Adressen ermöglichen. Also nicht mehr eine Abbildung Nachname->Adresse sondern Nachname->Liste von Adressen.

Wobei ich ein Dictionary sowieso nicht für so sinnvoll halte, da Adressen eigentlich eher eine sortierte Liste sind. IMHO.
Graf Wasserrutsche
User
Beiträge: 37
Registriert: Donnerstag 17. Juli 2008, 06:59
Wohnort: Köln
Kontaktdaten:

Beitragvon Graf Wasserrutsche » Dienstag 7. Oktober 2008, 13:09

BlackJack hat geschrieben:Das ist auf jeden Fall der falsche Weg. Wenn man ständig Zahlen aus Zeichenketten raussuchen und hin- und her wandeln muss, dann macht man etwas falsch.

Wenn Du ein Dictionary von Nachnamen auf Adresseinträge haben willst, und es mehrere Einträge für einen Nachnamen geben kann, sollte man halt auch genau das tun: Als Wert mehrere Adressen ermöglichen. Also nicht mehr eine Abbildung Nachname->Adresse sondern Nachname->Liste von Adressen.

Wobei ich ein Dictionary sowieso nicht für so sinnvoll halte, da Adressen eigentlich eher eine sortierte Liste sind. IMHO.


Okay, dann weg von der Idee :)

Also quasi mit einem Dictionary so? :

dict = {nachname:liste mit den elementen einer person}

Muss ich dann in diesem Falle die Liste abgrenzen und sagen, z.B. die ersten sechs Werte sind Person Nr.1, die nächsten sechst Person Nr.2 etc., oder kann ich in einem Dictionary auch einem Wert, also hier dem Nachnamen mehrere Listen (jeweils eine Liste pro Person) zuweisen?

Ich versteh nicht, wie man das sonst nur mit Listen machen könnte. Und wenn man pro Person z.B. eine Liste macht, wie würde man das dann pickeln?
BlackJack

Beitragvon BlackJack » Dienstag 7. Oktober 2008, 13:35

Wieso willst Du eine einzelne Person jetzt plötzlich als Liste modellieren? Du hast dafür doch schon `Person` als Typ!? Und wo jetzt das Problem beim `pickle`\n sein soll, verstehe ich auch nicht!?
Graf Wasserrutsche
User
Beiträge: 37
Registriert: Donnerstag 17. Juli 2008, 06:59
Wohnort: Köln
Kontaktdaten:

Beitragvon Graf Wasserrutsche » Dienstag 7. Oktober 2008, 13:44

BlackJack hat geschrieben:Wieso willst Du eine einzelne Person jetzt plötzlich als Liste modellieren? Du hast dafür doch schon `Person` als Typ!? Und wo jetzt das Problem beim `pickle`\n sein soll, verstehe ich auch nicht!?


Hm, ich glaube ich habe dich falsch verstanden. Jetzt steh ich wieder beim Problem davor.

Bis jetzt habe ich ja

dict = {person.name:person}

Wie meintest du das dann? Und ich ich versteh nicht wie man das mit Listen anstatt mit Dictionaries machen kann. Denke ich muss mir noch mehr über Listen etc. durchlesen, scheine das noch nicht richtig zu verstehen alles.
BlackJack

Beitragvon BlackJack » Dienstag 7. Oktober 2008, 14:24

Ich habe zwei Vorschläge gemacht. Einmal einen, bei dem das Dictionary Nachnamen auf Listen von Personen abbildet und dann, dass man das mit dem Dictionary ganz weg lassen sollte.

Vorschlag 1:

addresses = {person_a.name: [person_a, person_b, …]}

Für den Fall, dass ``person_a.name == person_b.name`` gilt.

Vorschlag 2: Das Dictionary in die Tonne kloppen und einfach eine (sortierte) Liste mit Personen führen.

addresses = [person_a, person_b, …]
Graf Wasserrutsche
User
Beiträge: 37
Registriert: Donnerstag 17. Juli 2008, 06:59
Wohnort: Köln
Kontaktdaten:

Beitragvon Graf Wasserrutsche » Donnerstag 9. Oktober 2008, 15:34

Okay, manchmal sieht man den Wald vor lauter Bäumen nicht :)

Habe jetzt die Idee mit den Listen ausgewählt und alles dementsprechend umgeschrieben und erweitert. Alle Funktionen funktionieren nun, jetzt macht mir die Sortierung Probleme.

Ich habe ein wenig geforscht und bin auf

Code: Alles auswählen

adressen = sorted(adressen, key=operator.itemgetter('name'))   

gekommen. Ich würde den Befehl als absolut logisch erachten, allerdings gibt er mir den Fehler ``TypeError: 'Person' object is unsubscriptable``.
Kann das damit zusammenhängen, dass ich nach einem String sortieren möchte? Oder habe ich im Grundgedanken noch einen Fehler?

Hier jetzt der neue Code. Ist das jetzt besser umgesetzt?

Code: Alles auswählen

import pickle
import operator

class Person(object):
    def __init__(self):
        pass
   
def person_new():
    person = Person()
    person.vorname = raw_input('Vorname: ')
    person.name = raw_input('Nachname: ')
    person.geburtsort = raw_input('Geburtsort: ')
    person.strasse = raw_input('Strasse: ')
    person.hausnummer = raw_input('Hausnummer: ')
    person.plz = raw_input('PLZ: ')
    person.stadt = raw_input('Stadt: ')
    return person

def person_show(adressen):
    for x in adressen:
        nr = adressen.index(x)
        nr = int(nr)+1
        print str(nr) + ') ' + x.vorname + ' ' + x.name

def person_details(nr, adressen):   
    person = adressen[nr-1]
    print '\nVorname: ' + person.vorname
    print 'Nachname: ' + person.name
    print 'Geburtsort: ' + person.geburtsort
    print 'Strasse: ' + person.strasse
    print 'Hausnummer: ' + person.hausnummer
    print 'PLZ: ' + person.plz
    print 'Stadt: ' + person.stadt + '\n'
   
def person_delete(nr, adressen):
    adressen.pop(nr-1)
    dump(adressen)

def dump(adressen):
    file = open('personen/daten.pk1', 'wb')
    pickle.dump(adressen, file)
    file.close()

def load():
    file = open('personen/daten.pk1', 'rb')
    personen = pickle.load(file)
    file.close()
    return personen

def menu():
    print '1) Anzeigen'
    print '2) Hinzufuegen'
    print '3) Loeschen'
    print '4) Beenden'
    return int(raw_input())

def main():
    while True:       
        i = menu()
        adressen = load()
        if i == 1:     
            ### adressen = sorted(adressen, key=operator.itemgetter('name'))               
            person_show(adressen)
            i = int(raw_input('Welche Daten moechten Sie einsehen? '))
            person_details(i, adressen)                     
        elif i == 2:     
            adressen = load()
            person = person_new()
            adressen.append(person)
            dump(adressen)
        elif i == 3:
            person_show(adressen)
            i = int(raw_input('Welche Daten moechten Sie loeschen? '))
            person_delete(i, adressen)
        elif i == 4:
            break;
   
main()


So langsam komme ich der Sache dahinter, nur um so mehr man erfolgreich löst, desto mehr Fragen kommen irgendwie auf :) Nochmal Vielen Dank für die Hilfe.
BlackJack

Beitragvon BlackJack » Donnerstag 9. Oktober 2008, 15:57

Naja, es ist halt die falsche Funktion. Du willst kein Element, sondern ein *Attribut*, also nicht ``person['name']`` sondern ``person.name``. Versuch's mal mit `operator.attrgetter()`.

Allerdings könntest Du auch den `Person`-Objekten eine passende `__cmp__()`-Methode verpassen.
farid
User
Beiträge: 95
Registriert: Mittwoch 8. Oktober 2008, 15:37

Warum nicht anydbm, shelve oder gar ZODB?

Beitragvon farid » Donnerstag 16. Oktober 2008, 17:10

BlackJack hat geschrieben:Wieso willst Du eine einzelne Person jetzt plötzlich als Liste modellieren? Du hast dafür doch schon `Person` als Typ!? Und wo jetzt das Problem beim `pickle`\n sein soll, verstehe ich auch nicht!?


Ausserdem: wieso das ganze Dictionary bzw. die ganze Liste picklen? Das kommt mir bei grossen Adresslisten ziemlich ineffizient vor.

Besser waere es hier shelve zu nutzen, wenn die Adressendatensaetze einen eindeutigen Schluessel haben (z.B. eine Kundennummer). Dann kann man auf einzelne Datensaetze direkt zugreifen, ohne die ganze Adressliste erst in den Speicher zu entpickeln.

Oder man benutzt gleich die ZODB, und leitet Person von persistent.Persistent ab, und gut ist. ;) Das waere dann aber eine Dependency zu nicht-Standardmodulen (die man mit easy_install ZODB3 erst installieren muesste).
Benutzeravatar
Hyperion
Moderator
Beiträge: 7471
Registriert: Freitag 4. August 2006, 14:56
Wohnort: Hamburg
Kontaktdaten:

Re: Warum nicht anydbm, shelve oder gar ZODB?

Beitragvon Hyperion » Donnerstag 16. Oktober 2008, 18:38

farid hat geschrieben:Ausserdem: wieso das ganze Dictionary bzw. die ganze Liste picklen? Das kommt mir bei grossen Adresslisten ziemlich ineffizient vor.

Naja, bei dem Interface dürfte das ziemlich schwer werden, so große Datenmengen zu erzeugen ;-)
Graf Wasserrutsche
User
Beiträge: 37
Registriert: Donnerstag 17. Juli 2008, 06:59
Wohnort: Köln
Kontaktdaten:

Beitragvon Graf Wasserrutsche » Mittwoch 11. Februar 2009, 16:09

So, da jetzt etwas Zeit ins Land gegangen ist habe ich alles mal überarbeitet und ein wenig anders gestrickt und würde gerne eure Meinungen dazu hören. Habe ich den Grundgedanken OOP richtig umgesetzt, oder sind da noch viele Fehler drin?

[Edit (Leonidas): Code ausgelagert.]
Benutzeravatar
Hyperion
Moderator
Beiträge: 7471
Registriert: Freitag 4. August 2006, 14:56
Wohnort: Hamburg
Kontaktdaten:

Beitragvon Hyperion » Mittwoch 11. Februar 2009, 18:18

Lagere doch bitte so langen Code aus!
http://paste.pocoo.org/ wird hier gerne verwendet.

Ansonsten:
- keine Kommentare!
- Wozu die Klasse Addressbuch? Die macht nichts gescheites imho! Also weg mit der Klasse und das ganze als Funktionen implementieren.
- Code auf Modulebene
- die Methoden in Adressbuch würde ich umstellen vom Namen her: list_person statt person_list. Das ist doch viel intuitiver!
- die Menüauswahl kann man per Dictionary lösen:

Code: Alles auswählen

def foo():
    pass

def bar():
    pass

choice = {
    1: foo,
    2: bar
}

# und nun der Aufruf:
choice[1]()


Verbesserungen / Ideen:
- Interface komfortabler gestalten: Suche nach Personen ermöglichen, bei vielen Einträgen kann man die ID wohl schlecht vorher raussuchen! (also Anfangsbuchstabe, SQL-Codes für Erfahrene User)

- Gruppen einbauen: Jede Person kann in beliebig vielen Gruppen sein, jede Gruppe kann beliebig viele Personen beinhalten (also eine klassische n:m-Relation).

- Dementsprechend kann die Suche nach Personen dann eben auch über Gruppen gehen.

-VCard Im- und Export: http://de.wikipedia.org/wiki/VCard
Benutzeravatar
Hyperion
Moderator
Beiträge: 7471
Registriert: Freitag 4. August 2006, 14:56
Wohnort: Hamburg
Kontaktdaten:

Beitragvon Hyperion » Mittwoch 11. Februar 2009, 18:21

Noch was übersehen:

raw_inputs würde ich nicht in einen Konstruktor packen! Wie reagiert der, wenn bei der Eingabe was schief läuft? Hole Dir die Daten zuvor und übergib Sie dann dem Konstruktor!

Und nimm den Code mal aus den Postings - die Seite lahmt extremst! ;-)

Wer ist online?

Mitglieder in diesem Forum: 0 Mitglieder