Probelm mit Programm

[cz3kit]

Hallo, ich hab hier grad ein Problem, wo ich einfahc nicht weiter komme.
Hier erst mal der Code: http://paste.pocoo.org/show/163674/

Ich hab dabei folgendes Problem. Wenn sich das MainFenster öffnet und ich dann auf Zeige Alle Kunden klicke, werden mir alle Kunden angezeigt die in dem Dict von CustomerDB enthalten sind, angezeigt. Ich habe jetzt die Methode removeCust so modifiziert, dass der Kunde mit mit der ID 2 gelöscht werden soll. wenn ich da also ebenfalls draufklicke, wird die Methode korrekt ausgeführt, aber wenn ich danach wieder auf Zeige alle Kunden klicke, dann wird der Kunde mit der ID 2 immer noch angezeigt, obwohl dieser nicht mehr da sein dürfte.

Ich hoffe das Problem ist verständlich.

[cofi]

Da ich leider nicht die Zeit habe mich da durchzuwuehlen, nur der Hinweis:

Code: Alles auswählen

if type(surname) != str:
    raise TypeError, "The surname has to be a string object"
else:
    self.surname = surname

liesse sich besser mit

Code: Alles auswählen

if isinstance(surname, str):
    self.surname = surname
else:
    raise TypeError, "The surname has to be a string object"

machen (durch das vertauschen der Suites ist es imho auch besser lesbar).

[derdon]

Mir sind vor allem drei Dinge aufgefallen:

• http://de.wikipedia.org/wiki/Duck-Typing
• http://www.python-forum.de/topic-13682.html
• http://www.python.org/dev/peps/pep-0008/

Außerdem wird das GUI-Toolkit nicht importiert (ist es tk?).

[Dauerbaustelle]

Du rufst `loadCustomers` auf, was das Dict überschreibt. (Du hast somit wieder die Werte aus der Pickle-Datei.)

Im Übrigen ist Backslash in Zeile 5 unnötig. Auf die Typchecks würde ich verzichten, und wenn du sie wirklich haben willst, nimm `isinstance`.

Außerdem hast du einige getter und setter drin, die komplett unnötig sind, die würde ich rausmachen.

Ohne dir zu nahe treten zu wollen: Du kommst aus der Java-Welt, nicht wahr? Mit Python schreibt man keinen solchen Code. Du versuchst zum Beispiel, Typen in Python zu erzwingen. Dafür ist die Sprache nicht ausgelegt. IMHO gilt bei Python-Programmen das Prinzip des Nicht-Dummen-Programmierers: Du solltest dich darauf verlassen, dass der Programmierer die richtigen Typen verwendet.

Von pseudo-privaten Variablen mit __ am Anfang rate ich dir auch ab.

Gruß

[cz3kit]

@cofi danke für den Tipp, das werde ich dan ändern.

@derdon

http://de.wikipedia.org/wiki/Duck-Typing

ich versteh nicht worauf du mit diesem link hinaus willst

http://www.python-forum.de/topic-13682.html

ich gehe davon aus, dass du das mit den Gettern und Settern meinst und wahrscheinlich, das mit den Unterstrichen.

Das Programm ist ein Schulprojekt und wir haben eine Vorlage bekommen, wo doe Getter uns Setter mit eingetragen sind. Das heißt ich muss die mit implementieren. Was die Unterstriche betrifft, wir sollen ebenfalls zwei UNterstriche machen, aber ich denke, mit den Argumenten was ihr in dem Text ist, werde ich wohl auch einen machen können.

Außerdem wird das GUI-Toolkit nicht importiert (ist es tk?).

Sorry, da hab ich den import vergessen. Ja, es ist Tk.

@Dauerbaustelle

Wenn ich das mit loadCustomer richtig verstehe, dann holl ich mir ständig den Inhalt aus der Datei, weil sich das im mainloop befindet.

Das mit isinstance ist mir neu, aber gut zu wissen, danke!

Das mit Gettern und Settern weißt ja jetzt bescheid

Ohne dir zu nahe treten zu wollen: Du kommst aus der Java-Welt, nicht wahr? Mit Python schreibt man keinen solchen Code. Du versuchst zum Beispiel, Typen in Python zu erzwingen. Dafür ist die Sprache nicht ausgelegt. IMHO gilt bei Python-Programmen das Prinzip des Nicht-Dummen-Programmierers: Du solltest dich darauf verlassen, dass der Programmierer die richtigen Typen verwendet.

Ich gehe davon aus, dass der Lehrer uns mehr so Java Style mithilve von Python eintrichtern will. Es scheint das es kein Sinn hat. Gut gut.

[derdon]

Ok, dann erkläre ich, was ich mit den ersten beiden Links sagen wollte (der dritte scheint anscheinend klar gewesen zu sein):

In Sprachen wie Python ist es schlechter Stil, Typen zu erzwingen (Dauerbaustelle sagte es bereits). Stattdessen sollte man einfach annehmen, dass der Programmierer die richtigen Typen verwendet. Vorteile sind u.a., weniger Code und mehr Flexibilität: Es können Klassen (in Python sind Typen und Klassen synonym zueinander) verwendet werden, die sich wie eine erwartete Klasse verhalten ohne, dass diese explizit als mögliche Klasse angebeben wurde (siehe den Wikipedia-Artikel).

In Python sollte man auf Getter und Setter wenn möglich komplett verzichten (Getter lassen sich nicht vermeiden, wenn der Rückgabewert dynamisch berechnet werden muss; dann sollte man auf das built-in property zurückgreifen).

[cz3kit]

@derdon ahso okej gut zu wisssen

Also ich hab das loadCustomer aus der init von CustomerDB genommen, aber er macht es denoch nicht. Mir ist aufgefallen, dass das Programm, wenn ich z.B. einen neunen Kunden hinzufüge, dann ist dieser nicht in dem Dictionary vorhanden, erst wenn ich speichere, das Programm beende und dann wieder öffne.

[BlackJack]

@cz3kit: Ich möchte mich da den Vorrednern anschliessen, welche die vielen Typprüfungen kritisiert haben. Wenn Du auf die und die Getter und Setter nicht verzichtest, dann ist Python die falsche Sprache für Dich. Beziehungsweise für den Lehrer. Sollte man ihm vielleicht mal vorsichtig sagen.

Ausserdem sind die Typprüfungen in `Person` ja fast alle doppelt vorhanden -- in der `__init__()` und in den Settern.

Bei den Prüfungen würde ich die originale Reihenfolge beibehalten, aber kein ``else`` verwenden. Das ``raise`` bricht die Ausführung an der Stelle ja sowieso ab.

Postleitzahlen sind kein Zahlentyp! Bei uns gibt es zum Beispiel führende Nullen die wichtig sind, bei Zahlentypen aber in der Regel einfach wegfallen, und in anderen Ländern können in der "Postleitzahl" auch Bindestriche und Buchstaben vorkommen.

Wenn man das Geschlecht schon einschränkt, sollte man es auch gleich normalisieren und nur eine Schreibweise (gross/klein) speichern.

Mal davon Abgesehen, dass die Methodennamen nicht PEP8-konform sind, ist `surname` *ein* Wort, also `get_surname()` oder halt `getSurname()`.

`__customers_dic` sollte kein Klassenattribut sein, so macht die ganze Klasse nicht viel Sinn weil man immer nur ein Exemplar davon gleichzeitig haben darf.

Laden, Speichern, und Erstellen von den DB-Objekten könnte man besser auf Methoden aufteilen. Der Dateiname sollte nicht zwingend im Exemplar gespeichert sein, oder zumindest "überschreibbar" sein, dann kann man so eine DB zum Beispiel auch einfach unter einem anderen Namen speichern als sie geladen wurde. Zum Beispiel um ab und zu mal eine Sicherheitskopie wegzuschreiben. Und ich stecke das Laden in der Regel nicht in die `__init__()`, denn es macht oft Sinn solche Objekte auch einfach nur für eine gewisse Zeit im Speicher existieren zu haben und entweder leer zu erstellen, oder mit schon im Speicher befindlichen Daten zu initialisieren. Dazu dann eine `staticmethod()` oder eine `classmethod()` als Alternative zu `__init__()` zum Erzeugen eines Exemplars aus einer Datei.

Du speicherst im Grunde den kompletten Zustand der DB, also würde es auch Sinn machen das Objekt als Ganzes zu speichern. Was Du wie ich gerade sehe auch machst, denn `CostumerDB.saveCostumers()` wird nie aufgerufen, dafür steckt in der GUI Code, der `pickle`t. Nicht schön. Das `Costumers` hätte man beim Namen der `save()`-Methode weglassen können. Das ist ja schon aus dem Klassennamen ersichtlich was da wohl gespeichert werden wird.

Wenn man in die Dokumentation nur "classdocs" reinschreibt, kann man sie auch gleich weglassen. Sinnlose Doku ist schlechter als gar keine.

Abkürzungen sollte man nur einsetzen, wenn sie allgemein bekannt sind. `Cust` für `Customer` ist das nicht.

Die Fehlermeldungen, die Du bei `IOError` ausgibst, sind ziemlich mutig. Wenn man eine Datei zum Beispiel zum Schreiben öffnet und das nicht funktioniert, heisst das nicht zwangsläufig, dass es die Datei nicht gibt. Das kann genausogut bedeuten, *dass* es sie gibt, man aber keine Rechte hat sie zu überschreiben, oder in das Verzeichnis zu schreiben, oder dass das Medium nur zum Lesen eingebunden wurde, oder…

Das was Du in der GUI an `set…`-Methoden hast, sind eigentlich eher `create…`-Methoden. Bei `set…()` erwartet man in der Regel ein Argument.

[cz3kit]

Wenn Du auf die und die Getter und Setter nicht verzichtest, dann ist Python die falsche Sprache für Dich. Beziehungsweise für den Lehrer. Sollte man ihm vielleicht mal vorsichtig sagen.

Ich hab es halt bis jetzt nicht anders gelernt, es schön das so viele das kritiseren, denn damit lern ich das es bei Python nicht nötig ist, im gegensatz zu Java.

Bei den Prüfungen würde ich die originale Reihenfolge beibehalten, aber kein ``else`` verwenden. Das ``raise`` bricht die Ausführung an der Stelle ja sowieso ab.

Ja stimmt, daran hab ich überhaupt nicht gedacht, dass es da ja schon abgebrochen wird.

Ich danke dir acuh für die weiteren Tipps. Ich werde diese auch noch bearbeiten.

Man lernt nie aus. Ein großes Danke an euch.

[sma]

Gegen eine Typprüfung der Attribute ist ja generell nichts zu sagen. Aber bitte nicht mit einzelnen getter- und setter-Methoden! Ein bisschen Metaprogrammierung ist hier IMHO besser:

Code: Alles auswählen

class Model:
    attributes = {}

    def __init__(self, **kw):
        for name, value in kw.items():
            setattr(self, name, value)
    
    def __setattr__(self, name, value):
        try:
            t = self.attributes[name]
        except KeyError:
            raise AttributeError(name)
        if not isinstance(value, t):
            raise TypeError(name)
        self.__dict__[name] = value

class Person(Model):
    attributes = {
        'first_name': str,
        'last_name': str,
    }

p = Person(first_name="Adam", last_name="Ant)

Stefan