Minesweeper (Tkinter)

[Astorek]

Hi @ all,

Tjoar... Habe hier mal einen Minesweeper-Klon in Tkinter geschrieben, den ich einfach mal hier vorstellen möchte und würde mich über Kritik und Verbesserungsvorschläge freuen^^.

Der Grund zum Programmieren des Spiels ist eigentlich ziemlich simpel: Ich wollte eine leichtgewichtige Minesweeper-Variante für Linux programmieren, da die Linux-Versionen davon das System mit Bibliotheken zumüllen, die ich sonst nie brauchen werde^^. Außerdem wollte ich sowieso schon immer mal Minesweeper nachprogrammieren, war mir aber nie sicher, wie ich das in rein prozeduralen Programmiersprachen (QBasic :\ ) machen sollte... Wie es der Zufall will, war ich grade sowieso dabei, meine Python-Kenntnisse aufzufrischen.

Als Ergebnis ist ein IMHO einigermaßen leicht durchschaubares Programm herausgekommen, über das sogar ich mich nach ein wenig Umschauen wieder zurechtfinde^^. Ich hab hier ja schon einige Threads bzgl. OOP und MVC erstellt (alle mehrere Monate alt), habe aber das Gefühl, es diesmal wirklich verstanden und einigermaßen sauber umgesetzt zu haben^^.

Zum Quellcode (und ein bisschen MVC-Fachchinesisch): Der ist in models, views, controls gegliedert mit entsprechenden Aufgaben. Der Viewer ist "nur" Beobachter, sendet Events aus und empfängt sonst ausschließlich vom Controller. Das Modell wäre ggf. komplett austauschbar... Und ich habe beim Programmieren selber gemerkt, wie leicht sich Änderungen im View umsetzen lassen, und diese mit minimalsten Änderungen im Controller auch gleich einsetzbar sind. Dass das Modell komplett unabhängig vom Viewer und vom Controller agiert, war auch enorm hilfreich (wenn man das Modell mit einer hingeschluderten und nur für Programmierer bedienbaren View komplett auf Herz und Nieren testen kann, erübrigt sich das beim "endanwendertauglichen" Viewer und Controller - wenn dann noch Fehler passieren, kann man sicher sein, dass diese nicht am Modell liegen)...

Zum Schluss fehlt zwar noch ein bisschen Feinschliff in der Bedienung, aber ansonsten bin ich mit dem Programm aber absolut zufrieden. Für mich momentan gleichzeitig das komplexeste und übersichtlichste, was ich bisher programmiert habe^^. (Sorry, ich hab in den letzten 3 Tagen fast nichts anderes gemacht außer programmiert, ich brauch das grade )

Bevor ich noch mehr seltsame Sachen schreibe, stelle ich mal den Quellcode hier rein^^.

Achja: Quellcode wurde unter Python 2.7 und 3.4 erfolgreich unter Windows 8.1 64bit getestet; ich hab grad weder Linux noch Macs zur Hand, sehe aber keinen Grund, weshalb es darauf nicht laufen soll.

Liste auf gist.github.com mit allen Quelldateien
Direktdownload, auch von gist.github.com (die generieren seltsame Dateinamen, einfach nicht beachten^^)

Gruß,
Astorek

EDIT: Ich schicke einfach noch einen kleinen Screenshot hinterher^^:

[Astorek]

Kein Feedback, Keine Verbesserungsvorschläge? Ist der Code so schlecht lesbar dass sich den niemand antun will? Oder so gut, dass niemandem ein Verbesserungsvorschlag einfällt? (Letzteres bezweifle ich natürlich, war ein Witz^^). Oder die Präsentation so abschreckend?

Vermutung: Ich gestehe, ich habe nicht ganz nach dem "Python-Styleguide" programmiert, z.B. in "camelCase" statt "camel_case" bei Methodennamen. Aber das ich nach einigen Tagen *gar keine* Antwort bekomme, überrascht mich dann doch ehrlichgesagt^^.

Ich würde mich freuen, falls mir zumindest jemand sagen könnte, ob ich auf einen guten oder auf einen schlechten Weg bin... Oder ob ich zusehr "javaesque" programmiert habe, falls das zutrifft^^...

Ich freue mich auf jedes ernsthafte Feedback. (und ich verspreche, das ist das erste und letzte Mal, dass ich den Thread hochhole^^)

[BlackJack]

@Astorek: Ich hatte nur mal kurz drüber geschaut und mir sah's zu „overengineered” aus. Und zu viel ums mal eben detaillierter zu kommentieren.

Ein Event pro Schwierigkeitslevel finde ich nicht so gelungen. Ein neues Level hinzufügen bedeutet das ganze Programm um ein Event erweitern zu müssen.

Von `list` würde ich nur erben, wie bei anderen Typen auch, wenn man die Methoden und Operationen auch benötigt, also nicht nur `append()` und das iterieren, sondern auch alles andere sollte IMHO Sinn machen.

Das `minesweeper`-Modul sieht ziemlich leer aus. Da alles so schön entkoppelt sein soll wundert es das dort nur der Controller vorkommt und beim ”Startpunkt” die drei MVC-Teile nicht ”zusammengestöpselt”, man sich an dieser zentralen Stelle gar nicht einfach alternative Implementierungen aussuchen könnte wenn es sie gäbe. Der `views`-Import wird gar nicht verwendet.

Die Models enthalten Farbwerte und eine ColorChanged-Event, wobei das doch bis zu einem gewissen Grad keine Modell-, sondern View-Fragen sind. Es gibt natürlich Spiele da kann man so etwas wie Farben nicht wirklich aus dem Modell heraushalten, Rot und Schwarz bei Roulette zum Beispiel, aber ich hätte vermutet bei Minesweeper müsste das eigentlich ganz gut möglich sein. Da sollte ein Feld im Modell bei MVC nicht ”Grau” sein, sondern beispielsweise ”verdeckt”. Wie das, und ob das dann Grau dargestellt wird, kann man im View entscheiden/hinterlegen.

`__repr__()` sollte eine Zeichenkette zurückgeben.

`__getPosByObject()` sieht sehr ineffizient aus.

Ebenfalls nicht effizient ist das erstellen von Zeichenketten durch wiederholtes ``+`` in Schleifen wie in `getPrintView()`. Zeichenketten sind unveränderbar, darum muss da in der Regel bei jedem Erweitern alles bisherige im Speicher umkopiert werden. Idiomatisches Python sammelt die Einzelteile in Listen oder erstellt sie per Generatorausdruck und benutzt dann die `join()`-Methode auf Zeichenketten um alles zusammenzufügen.

Warum wird in `getMines()` und `getFalsemarkedMines()` jeweils eine leere Liste erzeugt und an die Ergebnisliste angehängt, nur um diese leere Liste gleich im nächsten Schritt bedigungslos durch ein Tupel mit Koordinaten zu ersetzen!? Die Liste wird niemals irgendwie verwendet, und man hätte an der Stelle auch gleich das Tupel anhängen können.

Für Tupel mit Koordinatenangaben die Reihenfolge (y, x) zu verwenden ist IMHO ein wenig vewirrend.

Beim Erstellen der Schaltflächen für die Schwierigkeitsgrade werden in der Beschriftung Werte noch mal geschrieben die an ganz anderer Stelle im Quelltext als Werte in Funktionsaufrufen verwendet werden. Diese Wiederholung würde ich vermeiden und die Schwierigkeitsstufen als Daten speichern die man *einmal* im Quelltext schreiben und dann sowohl für die Beschriftung in der GUI als auch für das Erstellen eines Spielfelds verwendet, damit man die nur an *einer* Stelle im Programm ändern muss ohne Gefahr zu laufen die andere zu vergessen.

Eine Methode pro Farbe ist übertrieben. Statt die Farbe im Namen zu haben, würde man die besser als Argument übergeben. Weniger Code, mehr Flexibilität.

Die Implementierung von `disableAllButtons()` iteriert über `items()` verwendet dann aber nur den Schlüssel. Und statt der ”magischen” Indexzugriffe wäre es leichter lesbar wenn man die Werte gleich beim iterieren im Schleifenkopf an sprechende Namen binden würde.