Zurückgeben eines Wertes einer Klasse

[Ambriador]

Hallo an alle,

ich habe in meiner Klasse eine Funktion die etwas zurückgeben soll, was daraufhin mit der eigentlichen Eingabe überprüft werden soll, ob noch alles stimmt. Mache ich das mittels return oder muss ich da was anderes verwenden? Bekomme immer einen Error.

Code: Alles auswählen

    def in_treatment(self):
        if self.zimmer == []:
            return None
        else:
            return self.zimmer

Hier bekomme ich dann einen AssertionError

Code: Alles auswählen

assert(praxis.in_treatment() == patienten[0])

In self.zimmer und patienten[0] ist aber, so weit ich weiss, das gleiche.

[__deets__]

Das du den assertion-error bekommst ist doch nicht weiter ungewoehnlich - haengt halt davon ab, was in self.zimmer und patienten[0] ist.

Generell hast du einige Code-Smells verbaut:

Statt auf leere Liste zu pruefen, und dann None zurueck zu geben, gib gleich die leere Liste zurueck! Der Wahrheitswert davon ist der gleiche wie fuer None, sprich du kannst

Code: Alles auswählen

if liste: #nur wahr, wenn Eintraege in liste sind

machen, aber du kannst auch

Code: Alles auswählen

for item in liste:
     ...

machen, *OHNE* vorher auf None zu pruefen! Deutlich besser.

Und dann stellt sich die Frage, ob du ueberhaupt den Vergleich so durchfuehren solltest, wie du das gerade machst. Denn statt eine interne Datenstruktur rauszugeben bietet es siech eher an, die Frage ob der Patient im Zimmer ist *in* der Klasse zu beantworten.

Also

Code: Alles auswählen

praxis.in_treatment(patient)

Wie das dann bestimmt wird, kann man dann wunderbar der Klasse ueberlassen, und bei Bedarf auch aendern, ohne das man aufrufende Stellen anfassen muss.

[noisefloor]

Hallo,

Werte von Funktionen und Methoden werden mit `return` zurück gegeben, das ist richtig.

Wo jetzt sein Problem ist, lässt sich IMHO anhand der Codeschnipsel nicht sagen, weil das zu wenig ist. Wo kommt z.B. `patienten` her? Ist ja scheinbar kein Bestandteil der Klasse...
Ansonsten liegt die Vermutung nah, dass `self.zimmer` eine Liste ist, `patienten[0]` aber ein String? Ein Zahl? Jedenfalls kann das dann nie gleich sein.

Hilfreich ist es bei so was immer, sich alles mal per `print` ausgeben zu lassen. Dann siehst du, welche Werte `self.zimmer` und `patienten` haben.

Gruß, noisefloor

[Ambriador]

Der komplette code sieht so aus:

Code: Alles auswählen

patienten = [("Jan", 26), ("Marie", 13), ("Heinz", 20)]

class Surgery:
    def __init__(self):
        self.wartezimmer = []
        self.zimmer = []
        
    def enter_surgery(self, p):
        self.wartezimmer.append(p)
        
    def call_next_patient(self):
        self.zimmer.append(self.wartezimmer.pop(0))
        
    def in_treatment(self):
        if self.zimmer == []:
            return None
        else:
            return self.zimmer
        
    def treatment_done(self):
        self.zimmer.remove

EIgentlich sollte in beidem das gleich drin stehen.

[__deets__]

Deine letzte Zeile ist fehlerhaft. Dem remove fehlt die Klammer danach. Und deinen letzten Satz verstehe ich nicht. Was steht wo drin?

[noisefloor]

Hallo,

was willst du denn mit der `in_treatment` Methode erreichen? Zu prüfen, ob `zimmer` frei oder leer ist? Dann sollte die Methode aber `True` oder `False` zurück geben. Oder wer drin ist?

`zimmer` ist eine Liste und kann damit mehrere Einträge (=Patienten) aufnehmen - ist das gewollt?
Bei `self.zimmer.remove` fehlen die Klammer, richtig ist `self.zimmer.remove()`

`in_treatment` liefert eine Liste mit 1 (oder mehr) Elementen), `patienten[0]` liefert ein Tupel. Kann also nie gleich sein.

Was ist eigentlich der Sinn des ganzen? Ist das eine Übung? Oder was, was du aus Spaß an der Freude implentierst? Oder Produktiv-Coce? Das könnte man an einigen Stellen besser / robuster lösen.

Gruß, noisefloor

[Ambriador]

enter_surgery soll einen Patienten im Wartezimmer platzieren
call_next_patient soll einen Patienten vom Wartezimmer in das Behandlungszimmer versetzen
in_treatment soll prüfen ob ein Patient im Behandlungszimmer ist
treatment_done soll den Patienten aus dem Behandlungszimmer entfernen.

zimmer soll immer nur ein Objekt bzw Patienten aufnehmen, müsste dazu dann noch was in "call_next_patient" einbauen oder?

Ah ok, also könnte ich in "in_treatment" auf das erste Objekt in self.zimmer zugreifen, das sollte dann doch das passende Tupel sein oder?

[__deets__]

Zimmer waere dann vielleicht besser "op" oder wenn "behandlungszimmer", und dann sollte es gar keine Liste sein, wenn es es eh nur einen Patienten gleichzeitig beinhalten darf. Was du wiederum auch durch asserts innerhalb der Klasse pruefen solltest.

[Sirius3]

@Ambriador: wenn in `zimmer` jeweils nur ein Patient sein kann, ist es unsinnig, das als Liste zu speichern. Wenn es mehrere Behandlungszimmer gäbe, könnte man über ein Wörterbuch nachdenken; sonst solltest Du `zimmer` einfach `patient_in_treatment` nennen und das Patiententuple direkt dort referenzieren.

[snafu]

Eine Liste von links aus zu löschen, ist keine gute Idee. Denn dann müssen alle anderen Elemente nach vorne verschoben werden, was je nach Größe der Liste zu deutlich mehr Rechenaufwand führt. Für sowas ist eine verkettete Liste meist geeigneter. In Python findet man eine entsprechende Implementierung im collections-Modul:

Code: Alles auswählen

from collections import deque

class Surgery:
    def __init__(self):
        self.waiting_room = deque()
        self.current_patient= None
       
    def enter_surgery(self, p):
        self.waiting_room.append(p)
       
    def call_next_patient(self):
        try:
            self.current_patient = self.waiting_room.popleft()
        except IndexError:
            self.current_patient = None
       
    def in_treatment(self):
        return current_patient is not None
       
    def treatment_done(self):
        self.current_patient = None

[/me]

Eine Liste von links aus zu löschen, ist keine gute Idee. Denn dann müssen alle anderen Elemente nach vorne verschoben werden, was je nach Größe der Liste zu deutlich mehr Rechenaufwand führt.

An pop bin ich schuld. Ich hatte das bei Stackoverflow vorgeschlagen. Zu meiner Verteidigung: Da war es noch ein .pop() ohne Parameter, da der Anwendungsfall nicht genau definiert war.

[Ambriador]

Ah ja stimmt, dann macht es wirklich keinen Sinn "zimmer" als Liste zu machen.

@snafu vielen Dank, muss ich mir nachher mal genauer anschauen