AntMe Versuch...

[DeJe]

Ich bin gerade dabei mir ein paar Grundlagen von Python bezubringen. Mit mehr oder weniger Erfolg. Ich komme aus der Systemprogrammierung (Hardware nah) mit Assembler/C/C++ und habe mit OOP bisher wenig am Hut. Ebenso sind Simulationen und Spieleprogrammierung nicht mein Spezialgebiet, ist sicher unschwer zu erkennen. Nichtsdestotrotz fand ich einen AntMe-Klon interessant und unvorbelastet bezüglich Python und Simulation habe ich mich mal rangewagt.

sidle.py
sidle-gui.py

Ich glaube ich habe halbwegs PEP008 eingehalten und zumindest etwas pythonesisch programmiert. Die Klassen gehen noch etwas durcheinander, alles in einem File, wenig bis gar keine Fehlerprüfungen, ... aber das "Produkt" stellt auch nicht kein Final dar. Über Tricks und Tips würde ich mich freuen.

Ich hoffe die "Freaks" sind nicht zu sehr angewidert vom Sourcecode.

Edit: Natürlich ist das nur ein prove of concept und auch nur single player, wenn man so will. Es fehlen Interaktionen, Events, mehrere Völker, und und und.

[Leonidas]

Hallo DeJe, willkommen im Forum,

Ok, das wären meine Beobachtungen über den Code. Fass das bitte als Konstruktive Kritik auf, denn wie ich weiter unten schreibe ist der Code bis auf eben paar Sachen brauchbar. Ich habe nur die Angewohnheit von vielen Codereviews alles anzumängeln was mir beim überfliegen auffällt und denke auch dass ich darauf hinweisen sollte.

``__del__`` ist kein Destruktur wie du ihn etwa aus C++ kennst. In Python gibt es keine Destrukturen, daher machen Zähler wie viele Instanzen einer Klasse existieren wenig sinn, weil sie sowieso fast immer falsche Werte anzeigen. Ich wüsste auch gar nicht wieso mich das in einem Programm interessieren sollte, wie viele Instanzen es gibt.

Was noch: du überschreibst die Built-Ins ``list()``, ``dir()``, ``type`` in einigen Methoden, das sollte man lassen, weil es dann zu seltsam zu debuggenden Fehlern kommt wenn man dort irgendwann ``list()`` oder ``dir()`` verwenden will. Insbesondere wenn man 6 Monate später wieder reinschaut oder jemand anderes am Quellcode arbeitet.

Statt Gettern und Settern würde man für dynamisch zu berechnende Attribute Properties nutzen, triviale Getter und Setter macht man in Python hingegen gar nicht. Suche einfach mal im Forum oder in der Dokumentation danach.

Die ``while``-Schleife mit den Ticks würde ich eher gegen eine ``for``-Schleife mit ``xrange()`` ändern, das ist IMHO doch etwas klarer. ``psyco`` zwingend vorrauszusetzen ist auch nicht so das Wahre, das sollte auch optional sein.

Eine leere Liste ist übrigens ``False``, da muss man nicht nachprüfen ob ihre Länge null ist. Den wx-Code habe ich mir nicht angesehen.

Ansonsten: durchaus gelungener erster Versuch, die Verwendung von PEP8 hat zumindest bewirkt dass ich den Tab nicht sofort zugemacht habe, sondern da mal bisschen weiter geschaut habe. Aber bis auf die Kritikpunkte oben ist der Code weit über dem Durchschnitt des Codes den neue Mitglieder hier posten, von daher eben die paar Korrekturen und du kannst durchaus zufrieden sein.

P.S.: "proof"

[DeJe]

Konstruktive Kritik wie die deine ist sehr willkommen.
zu psyco: Wenn ich das nur im gui in main() importiere, funktioniert es auch für sidle.py bzw. für die Imports? Da war ich mir unsicher. Ich habe das aber jetzt geändert wie auf der Seite beschrieben:

Code: Alles auswählen

    try:
        import psyco
        psyco.full()
    except ImportError:
        pass

Eigentlich wollte ich auf solche speziellen Module (in der Lernphase) komplett verzichten, aber der Performance-Schub ist schon extrem spürbar und notwendig bei dieser Art Programm.

Die "Überschreibungen" der Klassen (list(), dir(), ...) sind natürlich gar nicht gewollt, habe ich komplett übersehen. Den Destruktor habe ich eliminiert, genutzt habe ich den Count ja sowieso nicht.
Auch die anderen Dinge (insbesondere Getter/Setter) schaue ich mir an. Schön den Schubs in die richtige Richtung bekommen zu haben.

[gerold]

Hallo DeJe!

Willkommen im Python-Forum!

Ich bin gerade dabei mir ein paar Grundlagen von Python bezubringen.

Das nehme ich dir nicht ab. Du müsstest sehr intelligent und/oder aufnahmefähig sein. Wenn das für dich zutrifft -- Hut ab.

- Ich habe nämlich noch keinen Anfänger gesehen, der gleich am Anfang kapiert, wie ein schönes wxPython-Programm aufgebaut sein soll und nicht sofort auf das hier http://wxpython.org/codeguidelines.php und auf die vielen IDs in manchen Beispielprogrammen reinfällt.

- Weiters teilst du die Datenstruktur sofort in ein zweites Modul, wodurch eine strikte Trennung zur GUI gewährleistet wird.

- Du verwendest ``if __name__ == '__main__'``, was ich bei Anfängern selten beobachten kann.

- Du importierst diszipliniert und verwendest Konstanten (sogar in Großbuchstaben).

- Weil kein Container benötigt wird, verwendest du sofort wx.Window statt wx.Panel.

- ``self.__class__.count`` --> so etwas müsste man einem OOP-Anfänger erst mal erklären.

- Du verwendest NewStyle-Klassen (object).

- Du verwendest aussagekräftige Objektnamen.

Sagen wir es mal so: Ich glaube, du hast dir die letzten Wochen alles zum Thema Python reingezogen was es zu finden gab.

Und jetzt zur Kritik.

Dein Programm sieht nicht so aus, als ob es ein Anfänger geschrieben hätte. Es gibt, aus meiner Sicht, auch kaum etwas zu bemängeln. Ein paar Punkte hat Leonidas bereits genannt.

sidle-gui.py

Im Namen von Python-Modulen sollte man Bindestriche vermeiden. Das erschwert das Importieren. Da du nur das GUI-Modul so genannt hast, dürfte das in deinem Fall keine Probleme verursachen. "sidlegui.py" fände ich persönlich besser.

Du beginnst den Namen der globalen Konstanten mit einem Unterstrich. Das ist nicht falsch -- nur etwas ungewöhnlich.

``if grid == None`` würde ich durch ``if grid is None`` ersetzen.

-----------

Trotzdem: Ich bin beeindruckt. Weiter so!

mfg
Gerold

[BlackJack]

@DeJe: Die Klasse `Coord` wird als Namensraum für Funktionen missbraucht. Das ist an sich schon nicht schön, dafür dann aber noch einmal den Umweg über ein Exemplar der Klasse zu gehen ist überflüssig. Wenn man in den Funktionen das `self` einfach weg gelassen hätte und sie zu `staticmethod`\s macht, wäre wenigstens die Absicht klarer, die dahinter steckt.

Das mit den `type`\s gefällt mir nicht so gut. Anstatt Tests mit Zeichenketten durch zu führen hätte ich eher über Methoden oder Attribute nachgedacht die Eigenschaften prüfen, zum Beispiel ob man etwas "essbares" vor sich hat oder so.

Ober und Untergrenzen von einem Wert kann man auch Testen ohne den Wert zweimal angeben zu müssen, also:

Code: Alles auswählen

newpos.real < 0 or newpos.real >= self.grid.width
# ->
not (0 <= newpos.real < self.grid.width)

`Creature.move()` ist mir persönlich zu lang und zu komplex. Da werden zu viele unterschiedliche Sachen in eine Methode geworfen. Vielleicht könnte man hier auch etwas planvoller einen Zustandsautomaten implementieren. So vom kurzen drüberschauen schliessen sich die drei separaten Boolean-Flags doch auch gegenseitig aus, oder? Mit `sugar` in Zeile 112 wird überhaupt nichts gemacht!? Womit die `KeyError`-Behandlung drum herum etwas fragwürdig wirkt, da kann man dann einfacher mit ``in`` testen ob der Schlüssel existiert.

[DeJe]

@Gerold, ich bin neu bei Python (ca. 1 bis 2 Wochen), nicht bei der Programmierung. Das mache ich schon seit Jahren. Treiber haben im Normalfall kein GUI, da braucht man eh, wenn überhaupt, ebenfalls ein separates Modul. So gesehen also nix Neues für mich.

Die angesprochenen Kritikpunkte sind soweit ausgeräumt bzw. geändert.

BlackJack hat den Finger auf der Wunde. Diese Type-Geschichte, Boolean-States und Zeichenkettenvergleiche gefallen mir auch nicht. Hatte bisher aber keine richtige Idee wie ändern. Unabhängig davon, ein Zustandsautomat fehlt, stimmt. Auch ein Nachrichtensystem fehlt ja komplett.

[Vingdoloras]

``__del__`` ist kein Destruktur wie du ihn etwa aus C++ kennst. In Python gibt es keine Destrukturen

Ich will ja nicht stören, aber es gibt in Python Destruktoren (falls du nich was andres meinst) und sie heißen auch ``__del__``. Daher macht es schon Sinn, einen Zähler für die Anzahl der Instanzen zu haben...

EDIT:
Gruß,
Vingdoloras

[Rebecca]

Die Betonung liegt auf wie du ihn aus C++ kennst.

Note that it is possible (though not recommended!) for the __del__() method to postpone destruction of the instance by creating a new reference to it. It may then be called at a later time when this new reference is deleted. It is not guaranteed that __del__() methods are called for objects that still exist when the interpreter exits.

PS: Suess, was ist denn eine De-Struktur?

[BlackJack]

Das ist vielleicht ein wenig Definitionssache, aber nicht-deterministische "Destruktoren" nennt man üblicherweise "finalizer" und nicht "Destruktoren". Aus dieser Sicht hat Python also tatsächlich keine Destruktoren.

[Vingdoloras]

Und wieder hab' ich was dazugelernt^^

[Leonidas]

Daher macht es schon Sinn, einen Zähler für die Anzahl der Instanzen zu haben...

Eben nicht, weil sie nicht deterministisch ist. Somit hast du eine hohe Warscheinlichkeit, dass dein Counter größer ist als die eigentliche Anzahl von Instanzen.

[DeJe]

Unabhängig von der Destruktor-Diskussion mal ein neuer Stand (__del__ war eh überflüssig).
Ich hoffe ich habe alle Hinweise beachtet und nix übersehen. Die Ants werden intelligenter und es existiert auch ein Dispatcher-System. Zwar primitiv aber es funtkioniert.

sidle.py
myants.py
sidlegui.py

Ich muß sagen die Sprache gefällt mir sehr gut.
Mich würde interessieren wie ihr die "enum"-Klassen (Category, MsgType) gelöst hättet. Ist die Lösung (die ich aus dem Net habe) bäh oder geht das so? Eine full featured enum Klasse wollte ich nicht (gibt es ja auch). Für die paar Enums reicht es auch so und es bleibt performant.
btw. Performance, da wären ein paar Tips nicht schlecht was man besser (schneller) machen kann.

Edit: Uups, da sehe ich gerade ich muß "live" noch in "life" umändern...

[DasIch]

Mich würde interessieren wie ihr die "enum"-Klassen (Category, MsgType) gelöst hättet.

Ich hätte einfach ein dictionary verwendet. Mehr sind diese Klassen auch nicht, nur der Zugriff ist anders.

[veers]

Ohne jetzt alles gelesen zu haben:

Code: Alles auswählen

obj.category == sidle.Category.FOOD

wäre da nicht sowas passender: isinstance(obj, Food)?

Und anstelle von

Code: Alles auswählen

objects = [obj for obj in self.grid.items \
                   if obj.category == sidle.Category.FOOD]
        dc.SetBrush(wx.Brush("white"))
        dc.SetPen(wx.Pen("white", 1))
        for obj in objects:
            dc.DrawCirclePoint((obj.pos.real,obj.pos.imag),2)

Code: Alles auswählen

for item in self.grid.items:
    item.draw(dc)

[DeJe]

a) Nein. Wenn später zusätzliche "Spieler"-Klassen dazu kommen wird es kompliziert welche Viecher z.B. ein Bug angreifen kann.
Ich habe das jetzt mal über set() versucht. Das Ergebnis war sehr frustrierend, in Bezug auf Performance. Ich bleibe erstmal bei INT und nutze '&' und '|'. Das hat den Vorteil das ich mich damit bestens auskenne und es verdammt schnell ist.

b) Ja, im ganzen Progamm kann Einiges optimiert (refaktoriert) werden.

[veers]

a) Nein. Wenn später zusätzliche "Spieler"-Klassen dazu kommen wird es kompliziert welche Viecher z.B. ein Bug angreifen kann.
Ich habe das jetzt mal über set() versucht. Das Ergebnis war sehr frustrierend, in Bezug auf Performance. Ich bleibe erstmal bei INT und nutze '&' und '|'. Das hat den Vorteil das ich mich damit bestens auskenne und es verdammt schnell ist.

Wenn du solche Lowlevel Optimierungen magst solltest du C programmieren und nicht Python

[DeJe]

Ja, da hast du sicher recht.
Aber bei dieser Applikation sei es mir vielleicht verziehen mit & und | zu arbeiten statt Sets. Als nächstes Projekt suche ich mir etwas was keine solch hohen Ansprüche an die Performance stellt.

[veers]

Ja, da hast du sicher recht.
Aber bei dieser Applikation sei es mir vielleicht verziehen mit & und | zu arbeiten statt Sets. Als nächstes Projekt suche ich mir etwas was keine solch hohen Ansprüche an die Performance stellt.

Ich frage mich mehr warum die Bitfields und ifs an Stelle von Vererbung verwendest. Ein Bitfield an Stelle eines Sets kann ich noch als Legitim ansehen.

[DeJe]

Kannst du mir mal ein Beispiel zeigen wie du das genau meinst?
Sozusagen für jede Kategorie eine Stellvertreterklasse?
Wie kann man am schnellsten auf Zugehörigkeit einer Klasse prüfen? Instanz.__class__...?
Dann bekomme ich bei Vergleichen aber auch wieder Stringvergleiche rein.

Oder habe ich dich flasch verstanden?

[veers]

Kannst du mir mal ein Beispiel zeigen wie du das genau meinst?
Sozusagen für jede Kategorie eine Stellvertreterklasse?
Wie kann man am schnellsten auf Zugehörigkeit einer Klasse prüfen? Instanz.__class__...?
Dann bekomme ich bei Vergleichen aber auch wieder Stringvergleiche rein.

Oder habe ich dich flasch verstanden?

isinstance(obj, class)

Und das wären wohl keine reinen Tagging Klassen, sondern diese können / sollen auch Logik enthalten.

Aus:

Code: Alles auswählen

if obj.type == X:
something_for_x()
elif obj.type == Y:
something_for_y()

Wird:

Code: Alles auswählen

obj.something()

Es ist sogar möglich das der Code dadurch wieder vergleichbar schnell/schneller wird. Er ist jedoch sicherlich besser wartbar, erweiterbar und testbar!