Adressbuch, erster Versuch

Code-Stücke können hier veröffentlicht werden.
Benutzeravatar
ms4py
User
Beiträge: 1178
Registriert: Montag 19. Januar 2009, 09:37

Beitragvon ms4py » Mittwoch 11. Februar 2009, 18:31

Leider steckt hinter dem Programm kein gutes OOP Konzept.
Im Prinzip hast du eine Master/Gott Klasse "Adressbuch", die alles macht.
Deine Klasse Person ist so eingesetzt völlig unnötig, weil du sie nur einsetzt um den Input zu lesen.

Mein Vorschlag:
Bei Programmstart lädst du alle Personen aus der Datenbank und legst für jede eine Instanz an. Das Programm arbeitet dann nur mit diesen Instanzen, löscht und bearbeitet sie und legt ggf. neue an.
Bei Programmende wird die DB mit den Werten aus den Instanzen neu befüllt.

Außerdem:
- Beschränke deine Zeilen auf 80 Chars
- Starte das Programm nicht direkt, sondern so:

Code: Alles auswählen

if __name__ == "__main__":
  #db verbindung
  programm()

- verwende statt:

Code: Alles auswählen

            cursor.execute(sql)
            row = c.fetchall()
            for i in range(0,len(row)):
                print str(row[i][0]) + ") " + row[i][1] + " " + row[i][2]


lieber:

Code: Alles auswählen

cursor.execute(sql)
for row in cursor:
    print "%s) %s %s" % (row[0], row[1], row[2])

- Bei SQL Statements sollte man sich eigentlich angewöhen, KEYWORDS UPPERCASE und variablen lowercase.
- Dein Menü ist irgendwie aufgeteilt. Die Hälfte ist in der Adressbuch-Klasse und die andere Hälfte in der programm() Funktion. Nimm das doch zusammen in eine Funktion.
- Und auf normalem Weg kann man dein Programm natürlich auch nicht beenden...
Benutzeravatar
Hyperion
Moderator
Beiträge: 7472
Registriert: Freitag 4. August 2006, 14:56
Wohnort: Hamburg
Kontaktdaten:

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

ice2k3 hat geschrieben:Mein Vorschlag:
Bei Programmstart lädst du alle Personen aus der Datenbank und legst für jede eine Instanz an. Das Programm arbeitet dann nur mit diesen Instanzen, löscht und bearbeitet sie und legt ggf. neue an.

Wobei man darüber streiten könnte ;-) Aber wie ich selber auf Seite 1 schon anmerkte dürfte die RAM-Belastung bei einem Adressbuch grad noch vertretbar sein :-D
Benutzeravatar
Leonidas
Administrator
Beiträge: 16023
Registriert: Freitag 20. Juni 2003, 16:30
Kontaktdaten:

Beitragvon Leonidas » Mittwoch 11. Februar 2009, 18:53

``programm`` würde man übrigens eher ``main`` nennen, denn ``programm()`` ist so oder so kein guter Name für eine Funktion.

Ansonsten: das ``try-except`` ohne konkrete Ausnahme ist eine schlechte Idee, dann wenn man es unter Python 2.4 startet bekommt man als Fehler "Verbindung fehlgeschlagen", obwohl der eigentliche Fehler ja "Modul sqlite3 wurde nicht gefunden" ist. ``c`` ist ebenfalls ein schlechter Name für eine Connection. Ich denke ich würde die Connection auch nicht Modulglobal machen. ``person_list``, ``person_add`` etc. würde ich ``list``, ``add`` etc. nennen. Wie ice2k3 meinte ist die Klasse aber generell ungelungen, da sie eine Art Gott-Klasse ist und allen möglichen unnützen Kram macht. Sie sollte eigentlich nur ein Adressbuch repräsentieren und das ist alles. ``if row != None`` würde man eher ``if row is not None`` schreiben, außerdem überschreibst du die eingebaute Funktion ``id()``, sowas sollte man auch nicht tun.
My god, it's full of CARs! | Leonidasvoice vs Modvoice

Wer ist online?

Mitglieder in diesem Forum: 0 Mitglieder