Hallo, ich habe hier ein kleines Problem mit definierten Variablen:
http://paste.pocoo.org/show/83423/
Und zwar habe ich in Zeile 17 eine Variable definiert und möchte sie in Zeile 66, ohne das ich sie lokal neu definiere, verändern. Leider weiß ich nicht so recht, wie ich das machen kann. Also helft mir Bitte.
Grüße
Edit (BlackJack): Quelltext ausgelagert.
Definierte Variablen aus eingebetteter Funktion ändern
Zunächst einmal hat dein Code ein sehr eigenwillige Struktur. Zwar lässt Python es zu, kreuz und quer an (fast) beliebiger Stelle einzelne Funktionen zu definieren, aber MACHEN muss man das darum trotzdem nicht. Es macht den Code sehr unübersichtlich.
Mögliche Lösungen für dein Problem:
Arbeite mit Klassen und verwende ein Instanzattribut.
Oder: Tausche den benötigten Wert über einen Parameter aus.
Oder (quick & dirty): Verwende eine globale Variable (Stichwort "global")
Mögliche Lösungen für dein Problem:
Arbeite mit Klassen und verwende ein Instanzattribut.
Oder: Tausche den benötigten Wert über einen Parameter aus.
Oder (quick & dirty): Verwende eine globale Variable (Stichwort "global")
Das das etwas komisch aussicht wurde mir schonmal gesagt. Was könnte man deiner Meinung nach aus der Funktion herausnehmen?
Sollte ich evt. eine Klasse erstellen, die data hält, und über die ich die Inhalte von data ändern kann?
Sollte ich evt. eine Klasse erstellen, die data hält, und über die ich die Inhalte von data ändern kann?
-
BlackJack
@zipdrive: Die Methode ist auf jeden Fall zu lang und zu komplex. 34 lokale Namen und 31 Verzweigungen in einer Funktion kann doch kein Mensch mehr überblicken.
Die letzte Zuweisung in `add_edge()` hat keinen Effekt, weil `last_edge` ein lokaler Name in `add_edge()` ist, der danach sofort wieder verschwindet. Das gleiche gilt für die Zuweisungen an `closing_line_index` in `add_closing_edge()`
Was ist der Grund für das Dictionary `data`? Soweit ich das sehe wird das niemals als solches benötigt, sondern macht nur den Zugriff auf die drei enthaltenen Listen unnötig kompliziert.
Anstelle von ``while 1:`` ist ein ``while True:`` etwas ausagekräftiger.
``-1 * irgendwas`` kann man auch als ``-irgendwas`` schreiben.
``is`` und ``is not`` testen auf Objektidentität und sollten nur verwendet werden, wenn man das auch wirklich testen möchte. Wenn man Objekte nur auf (un)gleichheit testen möchte, verwendet man ``==``/``!=``. Insbesondere das so etwas wie ``len(links) is not 2`` überhaupt funktioniert ist Zufall bzw. ein Implementierungsdetail auf das man sich nicht verlassen sollte:
Falls `self.primitives.area_lines` ein Dictionary ist, kannst Du beim `max()` das ``.keys()`` weg lassen und sparst so eine temporär erstellte Liste mit den Schlüsseln. Ob ein Schlüssel in einem Dictionary enthalten ist, kann man auch mit dem ``in``-Operator testen, das ist schneller als `has_key()` und auch "polymorpher".
Ich habe so den Verdacht, das Zeile 132 nicht das tut, was Du denkst das es tut. Ausserdem gibt die Funktion entweder `True` oder `None` zurück, was ein eigenartiger Typmix ist.
Ein paar Zeilen sind länger als 80 Zeichen. Leerzeichen nach Kommata sind inkosequent gesetzt und die doppelten führenden Unterstriche sollten wahrscheinlich eher einfache sein.
PS: Ich hoffe Du hast `numpy` nicht nur wegen `pi` importiert, das gibt's nämlich auch in der Standardbibliothek im `math`-Modul.
Die letzte Zuweisung in `add_edge()` hat keinen Effekt, weil `last_edge` ein lokaler Name in `add_edge()` ist, der danach sofort wieder verschwindet. Das gleiche gilt für die Zuweisungen an `closing_line_index` in `add_closing_edge()`
Was ist der Grund für das Dictionary `data`? Soweit ich das sehe wird das niemals als solches benötigt, sondern macht nur den Zugriff auf die drei enthaltenen Listen unnötig kompliziert.
Anstelle von ``while 1:`` ist ein ``while True:`` etwas ausagekräftiger.
``-1 * irgendwas`` kann man auch als ``-irgendwas`` schreiben.
``is`` und ``is not`` testen auf Objektidentität und sollten nur verwendet werden, wenn man das auch wirklich testen möchte. Wenn man Objekte nur auf (un)gleichheit testen möchte, verwendet man ``==``/``!=``. Insbesondere das so etwas wie ``len(links) is not 2`` überhaupt funktioniert ist Zufall bzw. ein Implementierungsdetail auf das man sich nicht verlassen sollte:
Code: Alles auswählen
In [61]: a is 2
Out[61]: True
In [62]: a = 4711
In [63]: a is 4711
Out[63]: FalseIch habe so den Verdacht, das Zeile 132 nicht das tut, was Du denkst das es tut. Ausserdem gibt die Funktion entweder `True` oder `None` zurück, was ein eigenartiger Typmix ist.
Ein paar Zeilen sind länger als 80 Zeichen. Leerzeichen nach Kommata sind inkosequent gesetzt und die doppelten führenden Unterstriche sollten wahrscheinlich eher einfache sein.
PS: Ich hoffe Du hast `numpy` nicht nur wegen `pi` importiert, das gibt's nämlich auch in der Standardbibliothek im `math`-Modul.
-
BlackJack
Teile den Quelltext der Methode sinnvoll auf mehrere Funktionen und/oder Methoden auf.
Mir fällt auf, dass es da eine Schleife über range(2) gibt. Den Rumpf könnte man in eine Funktion auslagern, die dann zweimal (statt der Schleife) mit dem jeweiligen Parameter aufgerufen wird. Die Funktionen in dem while-1 kann man auch rausziehen (und die dann freien Variablen zur Parameterliste hinzufügen). Da add_edge() offenbar von nbr aus dem range(2) abhängt, kann man hier auch zwei Funktionen schreiben und in den erwähnten neuen Funktionsaufrufen das jeweilige Funktionsobjekt übergeben. Die dels hinter if edge_index kann man auch zu einer Funktion machen - schon aus Symmetrie zu add_edge.
Das sind nur einige Möglichkeiten, wie man dieses 158-Zeilen-Monster zerschlagen kann.
Stefan
Das sind nur einige Möglichkeiten, wie man dieses 158-Zeilen-Monster zerschlagen kann.
Stefan
Hallo, ich möchte das Thema noch einmal aufgreifen und euch meine Veränderungen zeigen:
http://paste.pocoo.org/show/84758/
Was meint ihr dazu? Gibt es Verbesserungsbedarf?
Grüße
http://paste.pocoo.org/show/84758/
Was meint ihr dazu? Gibt es Verbesserungsbedarf?
Grüße
-
BlackJack
Einrückung mit Tabs statt Leerzeichen ist unüblich und kann zu Problemen führen wenn man irgendwo Leerzeichen und Tabs aus versehen mischt.
`__slots__` ist ein Mittel um Speicher bei vielen Exemplaren von einem Typ zu sparen. Falls Du also nicht messbare Probleme mit dem Speicherverbrauch hast, solltest Du das lassen.
Den '\' um Zeilen fort zu setzen vermeide ich persönlich. "Logische Zeilen" hören erst auf, wenn alle Klammern geschlossen sind, also klammere ich Ausdrücke lieber, falls sie das nicht sowieso schon sind.
Das `Data.tmp`-Dictionary finde ich immer noch sehr ungwöhnlich. Das würde ich weglassen. In der Funktion, in der `Data.add_edge()` aufgerufen wird, bindest Du anscheinend alles was Du in das Dictionary steckst auch noch einmal an lokale Namen (`nextedge`, `vector`, und `norm`), die Werte könntest Du also auch als Argumente beim Aufruf übergeben, das ist viel transparenter.
In Zeile 80 wird auf `self.last_edge` zugegriffen, dass es soweit ich das sehe gar nicht geben dürfte und kurz darauf auf `edge_area_lines`, das es ebenfalls nicht gibt. Das gleiche gilt für `data` in den Zeilen 105, 151, und 152, sowie `new_index` in `_add_closing_line()`.
Hast Du den Quelltext auch mal laufen lassen!? Sag bitte nicht, Du schreibst so einen Monsteralgorithmus "am Stück" runter und hoffst, dass der dann ganz am Ende läuft und auch das tut was er soll.
In Zeile 166 ist auch immer noch ein Problem, dass der Code wahrscheinlich nicht dass tut was er soll. ``a or b in c`` wird so ausgewertet: ``a or (b in c)`` und ist nicht das gleiche wie ``a in c or b in c``
`__slots__` ist ein Mittel um Speicher bei vielen Exemplaren von einem Typ zu sparen. Falls Du also nicht messbare Probleme mit dem Speicherverbrauch hast, solltest Du das lassen.
Den '\' um Zeilen fort zu setzen vermeide ich persönlich. "Logische Zeilen" hören erst auf, wenn alle Klammern geschlossen sind, also klammere ich Ausdrücke lieber, falls sie das nicht sowieso schon sind.
Das `Data.tmp`-Dictionary finde ich immer noch sehr ungwöhnlich. Das würde ich weglassen. In der Funktion, in der `Data.add_edge()` aufgerufen wird, bindest Du anscheinend alles was Du in das Dictionary steckst auch noch einmal an lokale Namen (`nextedge`, `vector`, und `norm`), die Werte könntest Du also auch als Argumente beim Aufruf übergeben, das ist viel transparenter.
In Zeile 80 wird auf `self.last_edge` zugegriffen, dass es soweit ich das sehe gar nicht geben dürfte und kurz darauf auf `edge_area_lines`, das es ebenfalls nicht gibt. Das gleiche gilt für `data` in den Zeilen 105, 151, und 152, sowie `new_index` in `_add_closing_line()`.
Hast Du den Quelltext auch mal laufen lassen!? Sag bitte nicht, Du schreibst so einen Monsteralgorithmus "am Stück" runter und hoffst, dass der dann ganz am Ende läuft und auch das tut was er soll.
In Zeile 166 ist auch immer noch ein Problem, dass der Code wahrscheinlich nicht dass tut was er soll. ``a or b in c`` wird so ausgewertet: ``a or (b in c)`` und ist nicht das gleiche wie ``a in c or b in c``
danke, dass du dir das mal angesehen hast. das einige variablen nicht definiert sind ist darauf zurückzuführen, dass ich einfach nur das self davor vergessen habe.
ok, aber ein mußt du mir noch erklären. ich höre ständig irgendwo etwas in der art "monstercode". ist python etwa nur darauf konzipiert aufgaben zu erledigen, die mit nur ein paar zeilen umzusetzen sind?
grüße
ok, aber ein mußt du mir noch erklären. ich höre ständig irgendwo etwas in der art "monstercode". ist python etwa nur darauf konzipiert aufgaben zu erledigen, die mit nur ein paar zeilen umzusetzen sind?
grüße
-
BlackJack
Python hat keine Probleme damit unübersichtlichen, langen, schlecht aufgeteilten Quelltext mit kryptischen Namen auszuführen. Guter Quelltext ist aber nicht nur vom Rechner ausführbar, sondern für Menschen lesbar, verständlich und wartbar. Dazu gibt's halt ein paar Leitlinien, zum Beispiel das Funktionen nicht zu lang sein sollten, nicht zuviele Namen verwenden sollten, nicht zu tief verschachtelte Abfragen und Schleifen enthalten sollten, kommentiert sein sollten, usw.
Das Prädikat "Monstercode" hätte ich dem Quelltext auch in jeder anderen Sprache gegeben.
Das Prädikat "Monstercode" hätte ich dem Quelltext auch in jeder anderen Sprache gegeben.