@Kurmuro: Iiiih Typannotationen. *Wenn* man die macht, sollte man auch unbedingt regelmässig prüfen ob die auch zum Code passen beziehungsweise ob der Code zu den Typannotationen passt. Was verwendest Du denn dafür? ``mypy`` beschwert sich nämlich das `matrix` eine Typannotation braucht. Zumindest solange das *vor* `Cell` definiert wird.
Ich verwende da lieber das externe `attrs`-Package (`attr`-Modul), da muss man keine Typen angeben.
Verwende keine kryptischen Abkürzungen. Das `random`-Modul hat so einen schönen passenden Namen, es gibt keinen Grund das als `rnd` abzukürzen. Vor allem, da der Name dann auch nur ein einziges mal verwendet wird. Es gibt bestimmte Module die werden Abgekürzt importiert weil man sie im Code sehr oft benötigt, weil die Module sehr viele Namen zur Verfügung stellen die man in einem Modul verwendet. Das sollte aber die Ausnahme und nicht die Regel sein.
Auf Modulebene sollte nur Code stehen der Konstanten, Funktionen, und Klassen definiert. Das Hauptprogramm steht üblicherweise in einer Funktion die `main()` heisst. Was der Code auf Modulebene auf keinen Fall machen sollte ist irgendwelche Fenster öffnen oder eine Hauptschleife laufen lassen. Man muss ein Modul ohne solche Seiteneffekte importieren können. Zum Beispiel zur Fehlersuche, für automatisierte Tests, zum erstellen von Dokumentation, Analysewerkzeugen die den Code importieren, und so weiter. Um dem Fehler auf die Spur zu kommen wäre es zum Beispiel sinnvoll wenn man die Matrix auch manuell erzeugen könnte, statt zufällig und gezielt den Code mit verschiedenen Testmatrizen laufen lassen könnte.
Funktionen und Methoden sollten alles was sie ausser Konstanten benötigen als Argument(e) übergeben bekommen. `Cell.show()` braucht `screen` als Argument und `Cell.anzMinenErmitteln()` braucht `matrix`.
Namen werden in Python klein_mit_unterstrichen geschrieben. Ausnahmen sind Konstanten (KOMPLETT_GROSS) und Klassen (MixedCase).
Umlaute in Namen sollte man vermeiden. Python selbst kann das zwar, aber nicht alle Werkzeuge die man auf Python-Code loslassen kann, kommen damit zurecht. Die Mischung aus Deutsch und Englisch ist aber insgesamt sehr unschön. Wonach wird denn entschieden welcher Namen Deutsch und welcher Englisch ist?
Bei `cell_selected` verstehe ich nicht wie der Name zustande kommt und man kann am Namen auch nicht erkennen, dass es sich um eine Sequenz handelt, statt eines Einzelwertes.
Beim laden der Bilder wird immer das selbe Codemuster ausgeführt – das schreit nach einer Funktion. Für das laden der Nummernbilder bietet sich eine „list comprehension“ an.
`benachbarteFelder` ist eine Fehlerquelle die man überprüfen sollte, oder besser vermeiden in dem man diese Tupel nicht manuell hin schreibt, sondern durch Code erzeugt. Auch hier bietet sich wieder eine „list comprehension“ an.
Der Code der die Minen platziert ist problematisch. Falls die Anzahl der Minen grösser ist als die Anzahl der Felder, dann ist das eine Endlosschleife. Aber selbst wenn die Anzahl der Minen ins Spielfeld passt, läuft das ganze um so ineffizienter, je weniger Platz im Feld ist. Das ganze geht effizienter und sogar deutlich kürzer mit `random.sample()`.
Wenn in der `matrix` Objekte vom Typ `Cell` stecken, dann ist der Name `objekt` viel zu generisch.
Die Anzahl der angrenzenden Minen würde ich mit `None` initialisieren, damit es deutlich auffällt (`IndexError`) wenn man versucht `show()` zu verwenden *bevor* dieser Wert berechnet wurde.
Der Code in allen Zweigen bei `Cell.show()` macht letztlich das gleiche. Das einzige was sich unterscheidet ist das Bild was im Code verwendet wird. Da sollte man die Gemeinsamkeiten herausziehen, also in den Zweigen nur das Bild ermitteln und dann *einmal* danach den Code schreiben, der das dann ”blittet”.
Am Anfang von `anzMinenErmitteln()` sollte man die Anzahl auf 0 setzen, sonst kann man die Methode nicht mehr als einmal aufrufen ohne das die Daten inkonsistent werden.
Die relativen Positionen in der Schleife würde ich gleich an sinnvolle Namen binden statt da mit nichtssagenden Indexwerten drauf zuzugreifen.
Der Präfix `neue*` macht nicht wirklich Sinn. Es gibt im gleichen Namensraum ja keine `alte*`-Variante von den Namen von der man sich abgrenzen müsste.
Für das Testen der Grenzen von `spalte` und `zeile` braucht man nur ein ``and`` und verkettete Vergleiche wie sie in der Mathematik üblich sind.
Zwischenstand (ungetestet):
Code: Alles auswählen
#!/usr/bin/env python3
import random
from attr import attrib, attrs
import pygame # type: ignore
#
# TODO Clean up that german english naming mix in this module.
#
AUFLOESUNG = 500
RASTER = 9
MINEN_ANZAHL = 10
assert MINEN_ANZAHL <= RASTER ** 2, "too many mines"
ABSTAND = AUFLOESUNG // RASTER
def load_cell_image(filename):
return pygame.transform.scale(
pygame.image.load(filename), (ABSTAND, ABSTAND)
)
NORMAL_CELL_IMAGE = load_cell_image("Teil_10_ms_cell_normal.gif")
MARKED_CELL_IMAGE = load_cell_image("Teil_10_ms_cell_marked.gif")
MINE_CELL_IMAGE = load_cell_image("Teil_10_ms_cell_mine.gif")
NUMBER_IMAGES = [load_cell_image(f"Teil_10_ms_cell_{i}.gif") for i in range(9)]
#
# TODO Replace by code. Chained ``for``\s or `itertools.product()`.
#
BENACHBARTE_FELDER = [
(-1, -1),
(-1, 0),
(-1, 1),
(0, -1),
(0, 1),
(1, -1),
(1, 0),
(1, 1),
]
assert len(BENACHBARTE_FELDER) == len(NUMBER_IMAGES) == 8
@attrs
class Cell:
spalte = attrib()
zeile = attrib()
is_mined = attrib(default=False)
is_selected = attrib(default=False)
is_flagged = attrib(default=False)
anzahl_angrenzender_minen = attrib(default=None)
def blit(self, screen):
if self.is_selected:
image = (
MINE_CELL_IMAGE
if self.is_mined
else NUMBER_IMAGES[self.anzahl_angrenzender_minen]
)
else:
image = MARKED_CELL_IMAGE if self.is_flagged else NORMAL_CELL_IMAGE
screen.blit(image, (self.spalte * ABSTAND, self.zeile * ABSTAND))
def mimenanzahl_aktualisieren(self, matrix):
self.anzahl_angrenzender_minen = 0
for zeilen_delta, spalten_delta in BENACHBARTE_FELDER:
zeile = self.zeile + zeilen_delta
spalte = self.spalte + spalten_delta
if 0 <= zeile < RASTER and 0 <= spalte < RASTER:
if matrix[zeile * RASTER + spalte].is_mined:
self.anzahl_angrenzender_minen += 1
assert 0 <= self.anzahl_angrenzender_minen <= len(BENACHBARTE_FELDER)
def main():
pygame.init()
pygame.display.set_caption("Minesweeper")
screen = pygame.display.set_mode([AUFLOESUNG, AUFLOESUNG])
matrix = [Cell(n // RASTER, n % RASTER) for n in range(RASTER ** 2)]
for index in random.sample(range(RASTER ** 2), MINEN_ANZAHL):
matrix[index].is_mined = True
for cell in matrix:
cell.mimenanzahl_aktualisieren()
weitermachen = True
while weitermachen:
for event in pygame.event.get():
if event.type == pygame.QUIT:
weitermachen = False
for cell in matrix:
cell.is_selected = True
cell.blit(screen)
pygame.display.flip()
pygame.quit()
if __name__ == "__main__":
main()
Du solltest noch mal überprüfen ob `BENACHBARTE_FELDER` tatsächlich korrekt ist.
Und dann Testcode in einem weiteren Modul schreiben schreiben der das Modul mit dem Spiel importiert und nichtzufällige Matrizen prüft. Zum Beispiel ob bei einer komplett leeren Matrix das erwartete Ergebnis kommt. Und bei einer wo nur ein einziges Feld vermint ist. Wenn da nicht das gewünschte Ergebnis kommt, sieht man am tatsächlichen Ergebnis vielleicht ein Muster das einen auf das Problem im Code stösst.
Dazu müsstest Du mehr Code aus der Hauptfunktion in eigene Funktionen heraus ziehen. Zum Beispiel wirst Du für den Testcode das erstellen einer leere Matrix benötigen, und das aktualisieren der Anzahlen in den Zellen, nachdem Minen platziert wurden.
Um das Testen einfacher zu machen, würde es sich auch anbieten eine Funktion zu schreiben die aus einer Zeichenkette die das Minenfeld beschreibt eine Matrix mit Zellen zu erstellen. Und den umgekehrten Weg, eine Matrix bei der die Minen um die Zellen gezählt wurden, wieder als Zeichenkette umwandeln für einen einfachen Vergleich mit dem erwarteten Ergebnis.
Code: Alles auswählen
from minefield importiert minefield_from_bomb_map, minefield_to_string
def test_mine_counts(bomb_map, expected):
minefield = minefield_from_bomb_map(bomb_map)
assert minefield_to_string(minefield) == expected
if __name__ == "__main__":
test_mine_counts(
".........\n"
".........\n"
".........\n"
".........\n"
".........\n"
".........\n"
".........\n"
".........\n"
".........\n",
".........\n"
".........\n"
".........\n"
".........\n"
".........\n"
".........\n"
".........\n"
".........\n"
".........\n"
)
test_mine_counts(
"#........\n"
".........\n"
".........\n"
".........\n"
".........\n"
".........\n"
".........\n"
".........\n"
".........\n",
"#1.......\n"
"11.......\n"
".........\n"
".........\n"
".........\n"
".........\n"
".........\n"
".........\n"
".........\n"
)
test_mine_counts(
".........\n"
".#.......\n"
".........\n"
".........\n"
".........\n"
".........\n"
".........\n"
".........\n"
".........\n",
"111......\n"
"1#1......\n"
"111......\n"
".........\n"
".........\n"
".........\n"
".........\n"
".........\n"
".........\n"
)
# ...
Die Matrix/das Minenfeld selbst auch als Klasse zu modellieren würde sich hier anbieten. Dann kann man `__str__()` implementieren und eine Klassenmethode um aus einer Zeichenkette ein Minenfeld zu erstellen.
Für den Testcode könnte man sich dann auch mal `pytest` anschauen.
