AntMe Versuch...

Stellt hier eure Projekte vor.
Internetseiten, Skripte, und alles andere bzgl. Python.
DeJe
User
Beiträge: 39
Registriert: Sonntag 23. November 2008, 19:38

Sonntag 23. November 2008, 20:08

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
Administrator
Beiträge: 16024
Registriert: Freitag 20. Juni 2003, 16:30
Kontaktdaten:

Sonntag 23. November 2008, 20:35

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" ;)
My god, it's full of CARs! | Leonidasvoice vs Modvoice
DeJe
User
Beiträge: 39
Registriert: Sonntag 23. November 2008, 19:38

Sonntag 23. November 2008, 21:16

Konstruktive Kritik wie die deine ist sehr willkommen. :D
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. :roll: 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.
Benutzeravatar
gerold
Python-Forum Veteran
Beiträge: 5555
Registriert: Samstag 28. Februar 2004, 22:04
Wohnort: Oberhofen im Inntal (Tirol)
Kontaktdaten:

Sonntag 23. November 2008, 23:21

Hallo DeJe!

Willkommen im Python-Forum! :-)
DeJe hat geschrieben: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.
DeJe hat geschrieben: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
:-)
http://halvar.at | Kleiner Bascom AVR Kurs
Wissen hat eine wunderbare Eigenschaft: Es verdoppelt sich, wenn man es teilt.
BlackJack

Montag 24. November 2008, 00:01

@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
User
Beiträge: 39
Registriert: Sonntag 23. November 2008, 19:38

Montag 24. November 2008, 07:23

@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. :D

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
User
Beiträge: 53
Registriert: Sonntag 2. Dezember 2007, 18:25

Dienstag 25. November 2008, 12:56

Leonidas hat geschrieben:``__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 :D
Benutzeravatar
Rebecca
User
Beiträge: 1662
Registriert: Freitag 3. Februar 2006, 12:28
Wohnort: DN, Heimat: HB
Kontaktdaten:

Dienstag 25. November 2008, 13:04

Die Betonung liegt auf wie du ihn aus C++ kennst.
docs.python.org hat geschrieben: 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? :wink:
Offizielles Python-Tutorial (Deutsche Version)

Urheberrecht, Datenschutz, Informationsfreiheit: Piratenpartei
BlackJack

Dienstag 25. November 2008, 13:46

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
User
Beiträge: 53
Registriert: Sonntag 2. Dezember 2007, 18:25

Dienstag 25. November 2008, 15:04

Und wieder hab' ich was dazugelernt^^
Leonidas
Administrator
Beiträge: 16024
Registriert: Freitag 20. Juni 2003, 16:30
Kontaktdaten:

Dienstag 25. November 2008, 15:40

Vingdoloras hat geschrieben: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.
My god, it's full of CARs! | Leonidasvoice vs Modvoice
DeJe
User
Beiträge: 39
Registriert: Sonntag 23. November 2008, 19:38

Dienstag 25. November 2008, 21:35

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. :D

sidle.py
myants.py
sidlegui.py

Ich muß sagen die Sprache gefällt mir sehr gut. :D
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... :wink:
DasIch
User
Beiträge: 2450
Registriert: Montag 19. Mai 2008, 04:21
Wohnort: Berlin

Dienstag 25. November 2008, 22:19

DeJe hat geschrieben: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.
Benutzeravatar
veers
User
Beiträge: 1219
Registriert: Mittwoch 28. Februar 2007, 20:01
Wohnort: Zürich (CH)
Kontaktdaten:

Dienstag 25. November 2008, 22:36

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)
[url=http://29a.ch/]My Website - 29a.ch[/url]
"If privacy is outlawed, only outlaws will have privacy." - Phil Zimmermann
DeJe
User
Beiträge: 39
Registriert: Sonntag 23. November 2008, 19:38

Donnerstag 27. November 2008, 23:12

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. :D

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