Seite 1 von 1

Aufgabe mit Python

Verfasst: Samstag 5. April 2014, 18:14
von _nohtyp_
Kann sich mal jemand meine Lösung anschauen und sagen, was ich besser machen kann? Fange gerade erst mit Klassen an. (Aufgabe steht in der Datei)

https://github.com/toxinman/My-stuff

Danke

Re: Aufgabe mit Python

Verfasst: Samstag 5. April 2014, 18:24
von BlackJack
@_nohtyp_: Die Klasse macht generell keinen Sinn, das hätte man auch einfacher mit Funktionen haben können.

Es wird total inflationär alles mögliche als Attribute an das Objekt gebunden was an sich ja schon überflüssig ist. Und ausserhalb der `__init__()`-Methode werden neue Attribute eingeführt.

Die doppelten führenden Unterstriche machen keinen Sinn, da hier weder Mehrfachvererbung noch eine tiefe Vererbungshierarchie vorliegen. Implementierungsdetails werden mit *einem* führenden Unterstrich gekennzeichnet. Und die paar Attribute so zu verstecken und andere Werte die eigentlich *gar nicht* auf das Objekt als Attribute gehören dann ”öffentlich” zu machen, ist unsinnig

Die jeweiligen nummerierten ”Methoden” enthalten jeweils fast den gleichen Quelltext, liessen sich also durch jeweils *eine* Funktion ausdrücken, welche die Unterschiede als Argumente übergeben bekommt.

Re: Aufgabe mit Python

Verfasst: Samstag 5. April 2014, 18:32
von EyDu
Doppelte führende Unterstriche verursachen name-mangling und sind nicht für private Attribute gedacht. Wenn du kennzeichnen möchtest, dass es sich etwas um ein Implementierungsdetail handelt, dann verwende einen einfachen führenden Unterstrich. Hier stellt sich allerdings die Frage, warum "brennweite" und "gegenstandsweite" überhaupt privat sein sollten.

"bildweite_berechnen" könntest du besser als property implementieren. Dabei kannst du dann auch gleich die Funktion aufräumen und die Bildweide zurückgeben und nicht noch als Attribut speichern. Das ist ein unerwarteter Nebeneffekt. Hinzu kommt, dass nirgens in der __init__-Methode etwas von "self.bildweite" zu sehen ist, dass sollte zumindest auf None gesetzt werden. Irgendwo im Code Attribute einzuführen ist keine gute Idee. Das ist sehr unübersichtlich.

Wenn du schon die ganzen Attribute privat machst, dann kannst du dir auch die ganzen float-Aufrufe sparen. Das solltest du dann besser beim Setzen der Attribute machen und nicht bei jeder Berechnung. Auch sind die Verlängerungen der logischen Zeilen mittels Backslash nicht besonders schön. Wenn eine Zeile zu lang ist, dann solltest du passende Teilausdrücke rausziehen und denen Namen geben.

"bildweite1_berechnen" und "bildweite2_berechnen" sind nichtssagende Namen, lass dir hier mal etwas vernünftiges einfallen. Auch hier setzt du wieder Attribute, welche in __init__ nicht initialisiert wurden. Auch solltest du die Werte 0.2 und -0.1 als Konstanten festlegen. Da die beiden Funktionen identisch sind, kannst du sie auch zusammenfassen.

Bei "entfernung1_berechnen" und "entfernung2_berechnen" wieder das selbe Spiel. Setzen von Attributen, aufteilen in Teilausdrücke und absolut identischer Code. Lässt sich wieder vereinfachen.

"intervall_berechnen" ist schrecklich grausam. Wieder doppelte führende Unterstriche welche die bereits gesetzten Attribute ohne Unterstriche verdoppeln und im Prinzip nur eine Extramethode für Aufgaben, welche alle in der __init__ erledigt werden sollten.

Re: Aufgabe mit Python

Verfasst: Samstag 5. April 2014, 18:36
von darktrym
Statt ein Backslash kannst du eine Klammer nutzen.

Re: Aufgabe mit Python

Verfasst: Samstag 5. April 2014, 18:48
von BlackJack
Nachtrag: Der Rückgabewert von der `bildweite_berechnen()`-Methode wird nirgends verwendet. Dieses Ergebnis zurückgeben *und* an ein (öffentliches) Attribut binden ist eine total schräge und unübersichtliche API. Das machst Du ja mit allen ”Methoden” so. Keines von den in `intervall_berechnen()` eingeführten Attributen wird jemals verwendet. Das heisst all die anderen Methoden geben etwas zurück was nie verwendet wird.

Ungetestet:

Code: Alles auswählen

def bildweite_berechnen(brennweite, gegenstandsweite):
    return (
        (brennweite * gegenstandsweite * 1000.0)
            / (gegenstandsweite * 1000.0 - brennweite)
    )


def entfernung_berechnen(brennweite, bildweite):
    return ((brennweite * bildweite) / (bildweite - brennweite)) / 1000


def intervall_berechnen(brennweite, gegenstandsweite):
    bildweite = bildweite_berechnen(brennweite, gegenstandsweite)
    entfernung1 = entfernung_berechnen(brennweite, bildweite + 0.2)
    entfernung2 = entfernung_berechnen(brennweite, bildweite - 0.1)
    return entfernung2 - entfernung1


def main():
    print 'Intervall: {0} m'.format(intervall_berechnen(300, 50))


if __name__ == '__main__':
    main()