Seite 2 von 2

Verfasst: Donnerstag 9. Juli 2009, 22:19
von Leonidas
Ja; was denn auch sonst :)

Verfasst: Freitag 10. Juli 2009, 00:05
von feldmaus
Ich habe meinen Code mal neu hochgeladen. Kann mir Jemand mal erklären wie das mit der Ignore-Variante in der Quelle,
http://rw7.de/ralf/inffaq/polygon.html
gemeint ist ? Speziell folgender Satz:
Die Idee ist, anders zu zählen. Nicht die Anzahl der Schnittpunkte der Kanten mit dem Strahl ist interessant, sondern es wird gezählt, wie oft der Polygonzug die Seite des Strahls wechselt.

Verfasst: Freitag 10. Juli 2009, 08:35
von hendrikS
Dein Code macht aber immer noch den Eindruck als hättest Du das Prinzip von OOP nicht wirklich verstanden. Auch die __ ändern da nichts. Hat auch so kleines bisschen Javasmell.

Verfasst: Freitag 10. Juli 2009, 09:11
von feldmaus
hendrikS hat geschrieben:Dein Code macht aber immer noch den Eindruck als hättest Du das Prinzip von OOP nicht wirklich verstanden. Auch die __ ändern da nichts. Hat auch so kleines bisschen Javasmell.
Du musst schon genau schreiben, was Dir nicht gefällt. Abgesehen davon ist dies nur ein Modul welches wo anders benutzt werden soll, dann wird daraus auch OOP. :-)

Verfasst: Freitag 10. Juli 2009, 09:25
von Leonidas
Das sind alles zusammengewürfelte Funktionen mit hässlichen, nicht PEP8-konformen Namen die aus keinem sinnvollen Grund in einer Klasse sind. So eine Struktur zum aufsammeln von Funktionen gibt es in Python auch, nennt sich Modul.

Auch der restliche Code mit der rumindexerei sieht auch nicht sonderlich klar aus. Ein erster Blick auf den Code sagt "Hä?" und das ist genau das was nicht passieren sollte.

Verfasst: Freitag 10. Juli 2009, 09:59
von feldmaus
Leonidas hat geschrieben:Das sind alles zusammengewürfelte Funktionen mit hässlichen, nicht PEP8-konformen Namen die aus keinem sinnvollen Grund in einer Klasse sind. So eine Struktur zum aufsammeln von Funktionen gibt es in Python auch, nennt sich Modul.
Das stimmt so nicht. Der Klassenname ist aussagekräftig und fängt mit einem großen Buchstaben an alles andere fängt mit einem kleinen Buchstaben an. Die Namen habe ich aus dem Algorithmus hergeleitet(Siehe Link) und sind auch aussagekräftig gewählt. Gearbeitet wird mit Übergabe von Argumenten und Rückgabewerten, was ja die Kapselung, die OOP mitunter ausmacht unterstreicht.
Leonidas hat geschrieben:Auch der restliche Code mit der rumindexerei sieht auch nicht sonderlich klar aus. Ein erster Blick auf den Code sagt "Hä?" und das ist genau das was nicht passieren sollte.
Die Indexerei ist unschön, aber leider fällt mir da zur Zeit nix besseres ein. Ich habe mal auf Kosten der Performance den Code lesbarer gemacht.

Verfasst: Freitag 10. Juli 2009, 10:09
von Dauerbaustelle
Wozu ist das in einer Klasse? Hast du denn das Klassenmodell verstanden?

Verfasst: Freitag 10. Juli 2009, 10:23
von Defnull
1) Deine Klasse hat keine Klassenvariablen, also keinen Zustand. Es würde keinen Sinn machen, sie zu instanziieren.
2) Deine Klasse hat keine öffentlichen Methoden. Man kann sie also von außen auch nicht benutzen.
3) Die Klasse hat einen Side-Effekt (print) der aus dem Klassennamen und der Dokumentation nicht ersichtlich ist.
4) "The Return type of this object is bool or an exception." ... Klassen haben keinen "Return type". Außerdem ist __nonzero__(self) ist nicht definiert, die Dokumentation lügt also.

Was du versuchst, ist das aufsplitten eines Problems in mehrere kleine Teilprobleme. Das ist auch gut und schön so, hat aber nichts mit Klassen oder OOP zu tun.

Warum nutzt du keine Funktion mit geschachtelten Hilfsfunktionen? Dann ist der ganze Hilfs-Kram so "privat" wie er nur sein kann und das alles ergibt deutlich mehr Sinn.

Code: Alles auswählen

def is_point_in_polygon(point, polygon):
    def helper_fast(...):
        ...
    def helper_slow(...):
        ...
    return helper_fast(...) and helper_slow(...)

def test_polygon():
    point1 = np.array([[4], [4]])
    poly1 = np. array([[3, 5, 5, 3, 3], [3, 3, 5, 5, 3]])
    ...
    print "Is the point ... within polygon ... ?"
    if is_point_in_polygon(point1, poly1):
        print "Yes"
    else:
        print "No"

Verfasst: Freitag 10. Juli 2009, 10:26
von Leonidas
feldmann_markus hat geschrieben:Das stimmt so nicht. Der Klassenname ist aussagekräftig und fängt mit einem großen Buchstaben an alles andere fängt mit einem kleinen Buchstaben an. Die Namen habe ich aus dem Algorithmus hergeleitet(Siehe Link) und sind auch aussagekräftig gewählt. Gearbeitet wird mit Übergabe von Argumenten und Rückgabewerten, was ja die Kapselung, die OOP mitunter ausmacht unterstreicht.
Der Klassenname ist Blödsinn: erklär mir mal was eine Instanz von "Pointtests" darstellen soll? Es hat keinen *semantischen* Wert (davon abgesehen würde man Klassen im Singular benennen, denn Klassen gibt es eine, aber mehrere Objekte einer Klasse. PEP8 sagt auch, dass man da für Namen CamelCase verwendet). Alles andere fängt mit Kleinbuchstaben an, aber ist camelCase, wo PEP8 doch gerade names_with_underscores sagt. Außerdem ist eine Klasse dazu da Zustand zu kapseln (so dass man eben nicht ständig den Polygon mitgeben muss) - du kapselst dort keinen interessanten Zustand sondern hast halt einfach eine Reihe von Funktionen, die komisch benannt sind und sinnloserweise in einer unnötigen Klasse stecken.

Kapselung von Daten und Methoden macht OOP aus, nicht die zwanghafte Verwendung von Klassen. Argumente und Rückgabewerte sind eher für die funktionale Programmierung typisch, aber auch da verwendet man keine unnötigen Klassen um Funktionalität zu Kapseln sondern auch eher Module.

Verfasst: Freitag 10. Juli 2009, 11:52
von feldmaus
Also wäre an dieser Stelle ein Modul ohne Klasse mit nur Funktionen ausreichend ?
Was ist an den Namen <_checkPointIsOnLine()> und <_checkbounds()> nicht verständlich ?
check sagt aus, dass hier was geprüft werden soll und was geprüft werden soll kommt danach, also ob der Punkt auf der Line ist und ob der Punkt innerhalb der x/y-Grenzen ist. Wie hättest Du das denn sonst formuliert ?

Verfasst: Freitag 10. Juli 2009, 12:03
von Dauerbaustelle
feldmann_markus hat geschrieben:Wie hättest Du das denn sonst formuliert
`point_is_on_line` oder `is_on_line` und `is_in_polygon` oder `in_polygon`.

Verfasst: Freitag 10. Juli 2009, 12:03
von cofi
feldmann_markus hat geschrieben:Also wäre an dieser Stelle ein Modul ohne Klasse mit nur Funktionen ausreichend ?
Ja. Eine Klasse soll ein *Objekt* beschreiben, man sollte es nicht als schlichten Namensraum missbrauchen.
feldmann_markus hat geschrieben:Was ist an den Namen <_checkPointIsOnLine()> und <_checkbounds()> nicht verständlich ?
Es wurde nie gesagt, dass sie unverstaendlich sind, sondern dass sie nicht PEP8 konform sind. Demnach sollten sie ``check_point_is_on_line`` (oder noch besser: ``is_on_line(point)``) und ``check_bounds()`` heissen.

Verfasst: Freitag 10. Juli 2009, 16:12
von Leonidas
feldmann_markus hat geschrieben:Also wäre an dieser Stelle ein Modul ohne Klasse mit nur Funktionen ausreichend ?
Ja, wenn man keine Funktionalität von Klassen nutzt, dann ist das ein gutes Indiz dafür, dass man keine Klassen braucht um das Problem zu lösen.
feldmann_markus hat geschrieben:Wie hättest Du das denn sonst formuliert ?
``point_is_on_line`` etwa...