ist das hier eigentlich objektorientiert oder wirr ?

Wenn du dir nicht sicher bist, in welchem der anderen Foren du die Frage stellen sollst, dann bist du hier im Forum für allgemeine Fragen sicher richtig.
ippurk
User
Beiträge: 61
Registriert: Mittwoch 8. Juli 2009, 20:40

Hallo,

ich baue hier ja immer noch an meiner Miethausverwaltung. Bin das ganze jetzt mal wie folgt angegangen:http://paste.pocoo.org/show/132155/.
Es gibt also ein Haus, Wohnungen und Mieter. Und ein Hauptdict der Häuser, sowie eins der Mieter. (An was sollten die "Hauptdicts" eigentlich in meinem Fall normalerweise gebunden werden?)

Erst mal die Frage, würdet ihr das ähnlich machen ?

Und dann ein kleines Problem: In Zeile 68 soll also bei Erstellung des Mieters, falls eine Wohnung angegeben ist, der Mieter in die Liste aller Mieter, die je in der Wohnung gewohnt haben, eingetragen werden. Nur leider wird der bei allen Wohnungen in die tenants Liste gepackt und ich kapier einfach nicht, warum.
Benutzeravatar
cofi
Python-Forum Veteran
Beiträge: 4432
Registriert: Sonntag 30. März 2008, 04:16
Wohnort: RGFybXN0YWR0

Wirr. Die Klasse ``App`` ist ueberfluessig, gleichzeitig werden die beiden anderen Klassen implizit daran gebunden, vor allem daran, dass es nur ein App-Exemplar gibt und das auch noch ``app`` heisst.
Die beiden Klassen solltest du entweder davon abkoppeln und dich bei der Anwendung der Exemplare um die Verwaltung kuemmerst oder (was ich nicht so gut finde) der Initialisierung mitgeben, wohin sie sich verwalten soll.

Mit der Aufdroeselung faellt auch 68- weg und wenn so viele Punkte in einer Zeile vorkommen, sollte man sich schon fragen, was man da eigentlich tut und ob das nicht besser geht ;)

Daneben sind die Docstrings sehr nichtssagend.
BlackJack

So beim kurzen drüberschauen gibt's zwei grosse Schnitzer:

1. `app` ist global. Lass mal den Quelltext unter der ``if __name__``-Abfrage in einer Funktion verschwinden -- dann kannst Du auf `app` nicht mehr so einfach zugreifen. Sollte man so auch nicht können dürfen.

2. Defaultargumente werden genau einmal beim erstellen der Funktion oder Methode ausgewertet. Das ist Dein beschriebenes Problem mit `tenants`. Und das hast Du prinzipiell auch an den Stellen wo Du leere Dictionaries als Defaultwerte verwendest -- die gibt's jeweils nur einmal für alle Exemplare der Klasse.
jerch
User
Beiträge: 1669
Registriert: Mittwoch 4. März 2009, 14:19

Also ich finde auch Deine interne Referenziererei nicht so gelungen. Das das Flat-Exemplar wegen der "von hinten durch die Brust"-Referenz noch lebt, obwohl es in main() nicht gebunden wurde, verschleiert doch mehr und trägt nicht zur Durchsichtigkeit der Objekthierarchie und -API bei.
sma
User
Beiträge: 3018
Registriert: Montag 19. November 2007, 19:57
Wohnort: Kiel

ippurk, hier mein Versuch, das ganze etwas aufzuräumen.

Füge nicht irgendwelche Objekte als Seiteneffekt im Konstruktor zu einer globalen Liste hinzu, sondern mache das Hinzufügen explizit. Das `add_home(Home(...))` ist noch redundant, hier könnte man das `Home`-Exemplar erst in `add_home` erzeugen, doch dann hätte ich deinen länglichen Konstruktor kopieren müssen, wozu ich keine Lust hatte. Beachte auch den Einsatz des "Builder"-Pattern. Durch das Zurückgeben von `self` kann ich die einzelnen `add_XXX`-Methoden alle aneinanderhängen.

Beachte, dass es NICHT richtig ist, veränderbare Objekte als Standardwert in der Parameterliste zu benutzen. Ich benutze stattdessen ausdrücke wie `flats or {}` als Initialisierung.

Ich würde `ident_nr` lieber `home_nr` nennen, um konsistent mit `Flat` und `Tenant` zu sein und noch besser wäre `no` denn mir scheint `nr` keine englischsprachige Abkürzung zu sein.

Deine Modellierung von `Tenant` ist IMHO falsch, denn entweder sind diese unabhängig von den Mietwohnungen und dann macht Einzugsdatum und Auszugsdatum keinen Sinn oder aber es sind reine Datenobjekte, die an einer Mietwohnung hängen, dann haben sie aber keine übergreifende Identität (`tenant_nr`). Ich habe daher den Code, der versucht, einen Mieter an eine Mietwohnung zu hängen, einfach gelöscht.

Allgemein denke ich, dass das Objektmodell zu sehr von einer relationalen Datenbank-Denke beeinflusst ist. Statt eines `auxiliary_costs_key`, der ja wohl ein Schlüssel in eine andere Datenbanktabelle sein soll, würde ich hier ein Objekt warten, welches das notwendige Verhalten hat -- außer, man will das sowieso in einer RDB ablegen und versucht gar nicht erst, ein Objektmodell zu entwerfen.

Auch wenn Python nicht statisch getypt ist, finde ich es bei Objektmodellen praktisch, sich über die Datentypen Gedanken zu machen und das sogar zu dokumentieren. Vielleicht so:

Code: Alles auswählen

class Home(Model):
    """
    no = int
    street = str
    postcode = str
    flats = {int:Flat}
    housekeepers = [Housekeeper]
    """
Es sollte auch ohne Worte klar sein, das `flats` nun ein dict von `int`-Schlüsseln und `Flat`-Werten ist und `housekeepers` eine Liste (abweichend vom Original, damit ich ein Beispiel habe).

Stefan
ippurk
User
Beiträge: 61
Registriert: Mittwoch 8. Juli 2009, 20:40

ey, das ist ja besser als ne Fernuni oder Schule hier. Besten Dank für die viele Mühe.

Nach einiger, anscheinend notwendiger Grundlagenforschung und unter Einbeziehung möglichst vieler eurer Vorschläge, hier der neue Output:http://paste.pocoo.org/show/132217/.

Kommt mir jetzt alles schon ein wenig klarer vor. So besser ?
EyDu
User
Beiträge: 4881
Registriert: Donnerstag 20. Juli 2006, 23:06
Wohnort: Berlin

Im Moment ist die "App"-Klasse überflüssig, da sie eigentlich keine richtige Aufgabe erfüllt. Ich gehe mal davon aus, dass dort noch etwas Funktionalität hin soll, dann würde ich die Klasse aber auch noch entsprechend umbenennen. Wahrscheinlich in irgend etwas Richtung Verwaltung.

Die doppelten Unterstriche deuten übrigens nicht an, dass ein Attribut private sein soll, sondern dienen lediglich der Vermeidung von Namenskollisionen. Wenn du ausdrücken möchtest, dass jemand ein Attribut nicht einfach anfassen soll, da dort unerwartete Nebeneffekte versteckt sind, solltest du einen einfachen Unterstrich benutzen.
Das Leben ist wie ein Tennisball.
ippurk
User
Beiträge: 61
Registriert: Mittwoch 8. Juli 2009, 20:40

das mit dieser Appklasse ist mir selber auch noch nicht so ganz klar geworden, wobei da allerdings später so Aufgaben wie "gespeicherte Daten einlesen", "neue Daten speichern" und "Programm beenden" vielleicht ganz gut reinpassen würden. Ich wüsste aber auch nicht, wo die beiden dicts, nämlich tenants und homes sonst hingehören würden. Hab das hier schon mal gefragt und keiner hat dazu was genaues gesagt.

Und wegen der __, das hab ich hier:http://openbook.galileocomputing.de/pyt ... 12_001.htm gelesen, aber da gehts noch um Python 2.5, hat sich da was geändert, oder ist das Buch schlecht ?
EyDu
User
Beiträge: 4881
Registriert: Donnerstag 20. Juli 2006, 23:06
Wohnort: Berlin

Wenn dir nicht klar ist, was die App-Klasse macht, warum hast du dann eine? ;-) Wenn noch Funktionalität hinzu kommt, dann macht sie später (mit einem vernünftigen Namen) wahrscheinlich Sinn.

Die beiden Dictionaries kannst du auch als lokale Variablen halten. Klassen sind kein Selbstzweck in die alles reingeprügelt werden muss. Wenn keine Klasse notwendig ist, dann benutzt man eben keine.
ippurk hat geschrieben:Und wegen der __, das hab ich hier:http://openbook.galileocomputing.de/pyt ... 12_001.htm gelesen, aber da gehts noch um Python 2.5, hat sich da was geändert, oder ist das Buch schlecht ?
Das Bookmark zu dem Buch solltest du gleich mal wieder löschen. Genug Kommentare findest du zu dem Buch mit der Suchfunktion. Außerdem hat BlackJack ein wenig über das OO-Kapitel verfasst.
Das Leben ist wie ein Tennisball.
ippurk
User
Beiträge: 61
Registriert: Mittwoch 8. Juli 2009, 20:40

ah ok, verstehe. Werd mir Blackjacks Ausführungen durchlesen.

Wie meinst du das, die dicts als lokale variablen zu halten ? Wo muss ich die erstellen ? In meinem main def ? Und wie greife ich da dann später drauf zu ?
Nur deshalb gibts nämlich eigentlich erst mal diese Klasse App.
EyDu
User
Beiträge: 4881
Registriert: Donnerstag 20. Juli 2006, 23:06
Wohnort: Berlin

Richtig, in der main:

Code: Alles auswählen

def main():
    homes = {}
    tenants = {}

    homes[1] = Home(1, street="Big Street")
    homes[1].add_flat(Flat(homes[1], 23, auxiliary_costs_key=0.2))
    ...
Das Leben ist wie ein Tennisball.
sma
User
Beiträge: 3018
Registriert: Montag 19. November 2007, 19:57
Wohnort: Kiel

Wenn ich zwischen einem Singleton und zwei globalen Variablen wählen sollte, würde ich mich für das Singleton entscheiden. Eine globale Referenz weniger.

Nenne die `App` besser `Db` oder ähnlich, damit du nicht auf die Idee kommst, da noch ein UI hinzuzufügen. Sie repräsentiert das Datenmodell. Dieses sollte man immer von der Darstellung trennen. Dort sollte es vielleicht noch Methoden geben, um ein Haus oder einen Mieter basierend auf der Nummer zu finden:

Code: Alles auswählen

class Db:
    def get_home(self, no): return self.homes[no]
    def get_tenant(self, no): return self.tenants[no]
Die mit `__` beginnenden Eigenschaften und die zugehörigen Zugriffsmethoden sind IMHO unnötig. In Python würde man einfach sagen, dass der Programmierer Eigenschaften nicht setzen soll und gut ist. Es gibt keinen Compiler der diese Intension zur Übersetzungszeit prüft.

Der `AttributeError` ist glaube ich eher ein `ValueError` (bei Java würde ich wohl eine `IllegalStateException`` wählen) oder ich würde einfach `assert` benutzen und dann kommt da eben ein `AssertionError`. Aber es ist ja nicht ein Fehler, dass ein Attribut in dem Objekt fehlt.

Stefan
ippurk
User
Beiträge: 61
Registriert: Mittwoch 8. Juli 2009, 20:40

@sma: Mir kam dein erstgenannter Vorschlag auch gar nicht so schlecht vor, also eine App oder auch Db Klasse zu haben. Hab ich nur zur Zeit erst mal wieder rausgenommen. Sowas wird aber noch eingebaut werden müssen, ich muss ja auch später irgendwie die schon angelegten Daten beim Programmstart importieren können etc.

Wenn du mit UI ein User Interface meinst, das ist grad auch schon in Arbeit und kriegt ne eigene Klasse bzw auch ein eigenes Modul.

Wo du grad meinen AtrributError ansprichst, da haste Recht, ist jetzt umbenannt in ValueError, wobei das wohl auch nicht so ganz passt. Bräuchte wohl eher so eine Art LogicalError. Wobei, da fällt mir ein, in so einer Wohnung könnte ja auch eine WG wohnen. Hm.

Da du grad das Thema exceptions ansprichst, dazu eine kleine Zwischenfrage. Habe hier jetzt diese Datumsproblematik etwas erweitert. Und um eine sinnvolle Schnittstelle zur späteren Gui sicherzustellen, sollen also alle Daten in Form eines Strings übergeben werden können, dachte ich mir, da die Gui ja eh nur mit sehr einfachen Datentypen arbeiten kann, deshalb das hier:

Code: Alles auswählen

def _validate_date_input(date):
    """
    internal function.
    Converts a string dd.mm.yyyy into a datetime.date object
    Gives a syntax Error in case of input format is not a string as dd.mm.yyyy
    and the errors of datetime.date if anything else is detected
    """
    date = str(date)
    date = date.split(".")
    # are there 2 dots in the string ?
    if len(date) == 3:
        # does the year information contain 4 digits ?
        if len(date[2]) == 4:
            i = 0
            for entry in date:
                try:
                    # is it an integer ?
                    date[i] = int(date[i])
                    i += 1
                except ValueError:
                    raise SyntaxError("Date must be a string like dd.mm.yyyy")
        else:
            raise SyntaxError("Date must be a string like dd.mm.yyyy")
    else:
        raise SyntaxError("Date must be a string like dd.mm.yyyy")
    
    date = datetime.date(date[2], date[1], date[0])
    return date
andersrum gibts was ähnliches. Was einem Anfänger halt so in den Sinn kommt...
Sieht nur irgendwie ziemlich verschachtelt aus.

Macht das Sinn, werden da alle Ausnahmen abgefangen, geht das einfacher ??

aktueller Gesamtzustand liegt hier: http://paste.pocoo.org/show/132309/
Benutzeravatar
HWK
User
Beiträge: 1295
Registriert: Mittwoch 7. Juni 2006, 20:44

Verwende doch datetime.strptime(date_string, '%d.%m.%Y') und für die andere Richtung date.strftime('%d.%m.%Y').
MfG
HWK
Zuletzt geändert von HWK am Montag 3. August 2009, 14:43, insgesamt 1-mal geändert.
ippurk
User
Beiträge: 61
Registriert: Mittwoch 8. Juli 2009, 20:40

mist, aber danke
Benutzeravatar
HWK
User
Beiträge: 1295
Registriert: Mittwoch 7. Juni 2006, 20:44

Warum Mist? Weil Du es nicht selbst gefunden hast?
ippurk
User
Beiträge: 61
Registriert: Mittwoch 8. Juli 2009, 20:40

genau, hab die doku nur recht schnell überflogen und dann ne halbe Stunde gebraucht, um den Kram selbst zu schreiben. Und das wahrscheinlich eher mäßig gut. Naja, wieder was gelernt.
ippurk
User
Beiträge: 61
Registriert: Mittwoch 8. Juli 2009, 20:40

aber irgendwie ist in meinem Python 2.6 auch nur strftime vorhanden. Funktioniert auch. Nur strptime, da meckert er, das gäbe es wohl nicht. Was ist denn hier jetzt schon wieder los ? In der Doku steht: das gibts ab Version 2.5.
Benutzeravatar
HWK
User
Beiträge: 1295
Registriert: Mittwoch 7. Juni 2006, 20:44

Zeig mal Programm-Code und Fehlermeldung.
MfG
HWK
ippurk
User
Beiträge: 61
Registriert: Mittwoch 8. Juli 2009, 20:40

Skript:

Code: Alles auswählen

import datetime as datetime

date = datetime.date(2009, 12, 3)
date = date.strftime('%d.%m.%Y')
print date

date_1 = "23.4.09"
date_1 = datetime.strptime(date_str, '%d.%m.%Y')
Ausgabe:

Code: Alles auswählen

03.12.2009
Traceback (most recent call last):
  File "C:/.../datetime.py", line 8, in <module>
    date_1 = datetime.strptime(date_1, '%d.%m.%Y')
AttributeError: 'module' object has no attribute 'strptime'
Antworten