erste klasse, kritik erwünscht!

[gehirn]

hallo leute,
ich lese seit 2 wochen das galileo openbook zu python und habe heute eine erste klasse erstellt. sie heißt Vokabeldict und erbt von dictionary.
sie dient dazu, eine vokabeldatei zu verwalten. ich möchte sie euch eurer berechtigten kritk unteziehen, da ich noch ziemlich unsicher in sachen python bin. besonders die verwendung der globalen variable vokabelDatei finde ich etwas unschön. ich habe vorher java programmiert, da war das eleganter!

Code: Alles auswählen

class Vokabeldict(dict):
    vokabelDatei = None
    def __init__(self,dateiName):
        global vokabelDatei
        try:
            vokabelDatei = open(dateiName,"r+a")
            print "Datei %s geoeffnet!" %dateiName
            self.__einlesen()
        except IOError:
            print "Datei konnte nicht geoeffnet werden"


    def __einlesen(self):
        global vokabelDatei
        for line in vokabelDatei:
            line = line.strip()
            zuordnung = line.split(" ")
            self[zuordnung[0]] = zuordnung[1]
            
    def speichern(self):
        global vokabelDatei
        vokabelDatei.seek(0)
        for paar in self.iteritems():
            print >> vokabelDatei, paar
            

    def __del__(self):
        global vokabelDatei
        vokabelDatei.close()

vielen dank für eure anregungen!

gehirn

[DasIch]

Die Klasse ist überflüssig, du kannst dich nicht darauf verlassen das __del__ aufgerufen wird und das Openbook ist nicht gerade der beste Einstieg.

[EyDu]

Und außerdem:

Das "vokabelDatei" sollte nicht auf Klassenebene stehen, sondern über "self." angesprochen werden. Das es "global" gibt vergisst du am besten gleich.

Und eine Methode mit zwei Unterstrichen als private markieren zu wollen macht man auch äußerst selten, dazu reicht wirklich ein unterstrich.

Die ganze Zeit die Datei offen zu halten ist auch etwas ungewöhnlich. Öffne Sie doch einmal beim Einlesen und dann wieder beim Speichern. Dann wird sie auch jedes mal garantiert geschlossen. Statt dem "print >>" solltest du auch besser die write-Methode verwenden.

[CM]

Im Übrigen gibt es auch [mod]shelve[/mod]. Wäre vielleicht interessant für Dich.

Gruß,
Christian

[gehirn]

vielen dank, ihr habt mir wirklich geholfen. vor allem der tipp mit shelve

gruß gehirn

[BlackJack]

Ich habe den Verdacht Du hast Klassen in Python noch nicht verstanden, weil Du eine *Klassenvariable* `vokabelDatei` hast, die Du gar nicht verwendest und eine "globale" Variable mit dem Namen. Aber eben keine *Instanzvariable*, wie es sich eigentlich gehören würde und wie man das wohl auch in Java gemacht hätte.

Du rufst den Konstruktor der Basisklasse nicht auf. In diesem Fall macht das nichts, aber schaden tut es auch nicht.

Du vermischt Logik und (End)Benutzerschnittstelle. Wenn man ein `Vokabeldict`-Exemplar erstellt, hat man keine Möglichkeit fest zu stellen ob die Datei nicht geöffnet oder gelesen werden konnte, oder ob die Datei einfach nur leer war. Dafür wird etwas auf die Ausgabe geschrieben, was man da vielleicht gar nicht haben möchte.

Ausprobiert hast Du das anscheinend auch nicht wirklich, denn die Daten werden in einem Format geschrieben, welches beim Einlesen gar nicht verstanden wird.

Ein weiterer potentieller Programmfehler: Daten laden, Vokabeln aus Objekt entfernen, und wieder speichern funktioniert nicht. Die Datei wird nicht kleiner sondern enthält noch alte Vokabeln.

'r+a' ist kein gültiger Modus für Dateien.

Die gesamte Aufteilung des Quelltextes auf Methoden ist ziemlich daneben. Das wäre auch ein reichlich unpraktischer Entwurf in Java.

Letztendlich wäre der Code besser in zwei Funktionen zum Laden und Speichern eines `dict`-Exemplars aufgehoben:

Code: Alles auswählen

from __future__ import with_statement


def load(filename):
    with open(filename) as lines:
        return dict(line.split() for line in lines)


def save(filename, dictionary):
    with open(filename, 'w') as out_file:
        out_file.writelines('%s %s\n' % p for p in dictionary.iteritems())


def main():
    filename = 'test.dat'
    save(filename, {'ham': 'Schinken', 'egg': 'Ei'})
    print load(filename)


if __name__ == '__main__':
    main()

[Mad-Marty]

Blackjacks post ist schonmal richtig.

Um an einer Klasse zu üben probiere wie bereits erwähnt
eine klasse mit 2 oder 3 methoden zu konstruieren.

Im prinzip spricht nichts dagegen das einlesen im __init__ zu machen,
ich würde es aber in einer public "read" methode unterbringen und nur aufrufen. Es sei denn das entsprechende object ist ein "Einweg-Objekt", sprich es kann aufgrund vieler interner states eh nicht so einfach wiederverwendet werden - in diesem fall dann eben nur über __init__ eine file zuweisen lassen.

Global brauchst du nicht, wie bereits erwähnt.
Es ist etwas was nur sehr selten benötigt wird, und meist auch für kein gutes design steht. Ausnahmen für "MurksCode": make scripte für Crossplatform

Um den filenamen zu halten lös es doch am besten über

Code: Alles auswählen

def __init__(self, filename=None):
    self.filename = filename

Damit ist der filenamen für jede instanz unterschiedlich.
Bei dir würde JEDE instanz der klasse das gleiche "vokabelDatei" haben.