@Riebers: Okay, Tipps:
Das Hauptprogramm steht üblicherweise in einer Funktion die `main()` heisst, dann braucht man da auch keinen Kommentar dran zu schreiben das es das Hauptrogpramm ist.
Einen Namen an `None` zu binden macht keinen Sinn wenn man den Namen gleich in der nächsten Zeile an einen anderen Wert bindet, ohne vorher etwas mit dem bisherigen Wert gemacht zu haben. Das `Spiel`-Exemplar überhaupt an einen Namen zu binden ist hier nicht nötig, denn das einzige was damit gemacht wird, ist die `spielen()`-Methode darauf aufzurufen. Was in der Regel ein Warnzeichen ist. Wenn die Klasse dann neben der `__init__()` nur *eine* Methode hat, dann ist es fast immer eine Funktion die unnötigerweise und kompliziert als Klasse geschrieben wurde.
Wenn man `Spiel` zu einer Funktion macht, dann besteht das Hauptprogramm nur noch aus dem Aufruf dieser Funktion, also ist `Spiel` als Funktion eigentlich schon das Hauptprogramm.
Der Kommentar beim `random.seed()`-Aufruf ist falsch. Man will ja gerade *nicht* immer die gleichen Werte haben. Der Aufruf an sich ist aber schon überflüssig, und bei nicht so guten Implementierungen sogar schädlich. Lass den Aufruf einfach weg.
Sehr viele Namen sind sehr schlecht gewählt. Namen sollen dem Leser vermitteln was der Wert dahinter im Programm bedeutet. Einbuchstabige Namen und kryptische Abkürzungen tun das sehr selten. Warum Du etwas erst `farben` nennst, nur um es dann ohne Not in `x` umzubennenen, ist mir ein Rätsel.
Wobei es sich ja gar nicht um Farben handelt, sondern um Zahlen. Ein passenderer Name wäre hier vielleicht `geheimnis`. Ob das dem Benutzer am Ende als Zahlen, Farben, oder Symbolbildchen präsentiert wird, ist an dieser Stelle ja erst einmal egal.
Beim Erstellen des Geheimnisses bietet sich eine „list comprehension“ an. Und ich würde mit den Zahlen bei 0 anfangen, denn Informatiker fangen beim Zählen mit 0 an. Wie man diese internen Werte dann dem Benutzer präsentiert, ist eine andere Frage. Denn bei einer GUI wirst Du ja wahrscheinlich Farben verwenden wollen und wenn man die in einer Liste speichert, dann ist die erste Farbe am Index 0 und nicht am Index 1.
Warum `Master` so heisst, verstehe ich nicht. Klassen repräsentieren Dinge im weitesten Sinn. Was soll denn ein `Master` sein?
In der `Master.__init__()` wird das `versuche`-Attribut gesetzt was nirgends verwendet wird. Damit ist die Klasse die `__init__()`-Methode los.
In der `kombination()`-Methode wird das Attribut `counter` neu eingeführt. Nach dem abarbeiten der `__init__()` sollte ein Objekt alle Attribute besitzen. Das Objekt sollte in einem sinnvollen, vollständigen Zustand sein.
Jetzt könnte man natürlich wieder eine `__init__()` einführen und dort `self.counter` beispielsweise an `None` binden, *aaaber* dieses Attribut wird sowieso nur in der `kombination()`-Methode verwendet, sollte also eigentlich gar kein Attribut sein, sondern nur ein lokaler Name.
*Dann* ist `kombination()` allerdings semantisch keine Methode mehr, denn man kann einfach das `self` weg lassen und es als Funktion aus der Klasse heraus verschieben.
`kombination` ist kein guter Name für eine Funktion oder Methode. Dafür wählt man in der Regel Tätigkeiten weil Funktionen und Methoden etwas tun. Also beispielsweise `erfrage_kombination()`, weil die Funktion eine Kombination vom Benutzer erfragt. `kombination` wäre ein passender Name für das Ergebnis der Funktion. Auf jeden Fall besser als `b`, was so überhaupt nichts aussagt. Beim Aufrufer könnte man den Rückgabewert dann zum Beispiel `tipp` nennen, statt `y`.
Der Wert von `counter` ist redundant, weil das immer gleich der Länge der Liste mit den Benutzereingaben ist.
In dem ``try``-Block wird viel zu viel gemacht. Das sollte möglichst nur das stehen, was auch die Ausnahme auslösen kann. Also in diesem Fall die Umwandlung in eine Zahl und das binden an einen Namen. Vielleicht noch die Eingabe, damit man nicht einen zusätzlichen Namen für das Zwischenergebnis benötigt. Zu ``try``-Blöcken kann man auch einen ``else``-Block schreiben, wo dann das rein kommt was nur passieren soll wenn die Ausnahme *nicht* aufgetreten ist.
Einen Wert erst an eine Liste anhängen, um ihn dann auf den Wertebereich zu prüfen und gegebenenfalls wieder aus der Liste zu löschen ist umständlich. Einfacher und leichter verständlich wäre es erst zu prüfen, und den Wert nur an die Liste anhängen, wenn er den Kriterien entspricht.
Kommen wir zur `Master.raten()`-Methode: Das ist im vorgefundenen Zustand schon keine Methode, sondern einfach nur eine Funktion die in eine Klasse geschrieben wurde. Verschieben wir sie heraus, dann ist `Master` nun leer und kann entfernt werden.
`raten()` trifft es nicht so ganz, denn das tut ja der Benutzer. Diese Funktion wertet eine geratene Kombination aus.
`a` und `b` sind wieder schlechte Namen. Hier könnte man auch wieder `geheimnis` und `tipp` verwenden. Statt eines Kommentars das `z_w` die Voll- und `z_s` die Halbtreffer enthält, könnte man die Variablen gleich `volltreffer` und `halbtreffer` nennen.
Indexwerte als Laufvariablen in Schleifen, die dann nur als Index in Sequenzen verwendet werden, sind in Python ein „anti pattern“. Man kann *direkt* über die Elemente von Listen & Co iterieren, ohne den Umweg über einen Index. Wenn man über zwei iterierbare Objekte ”parallel” iterieren möchte, gibt es die `zip()`-Funktion.
Kommen wir zur Auswertung innerhalb der Schleife mit ``if``/``elif``/``elif``. Was als erstes auffällt, ist das alle Bedingungen gleich anfangen. Diese erste Teilbedingung braucht man also nicht drei mal prüfen, sondern kann sie als eigenes ``if`` um das ganze setzen.
Der Teilausdruck `zaehler_a - zaehler_schwarz` kommt dort insgesamt vier mal vor, den könnte man vorher *einmal* berechnen.
In jedem Zweig wird am Ende das gleiche gemacht, also reicht ein ``if`` mit den ganzen Bedingungen entsprechend verknüpft. Falls ich mich nicht ”verrechnet” habe, müsste die Bedingung so aussehen:
Code: Alles auswählen
tmp = zaehler_geheimnis - zaehler_schwarz
if (
zaehler_geheimnis > 0
and (
(zaehler_tipp == zaehler_geheimnis and (tmp in [1, 2]))
or (zaehler_tipp > zaehler_geheimnis and tmp == 1)
or zaehler_tipp - zaehler_schwarz == 1
)
):
halbtreffer.append('o')
Was aber immer noch zu kompliziert ist. Ich verstehe es ehrlich gesagt auch nicht und ich möchte nicht wissen wie das aussehen muss wenn man statt vier beispielsweise zehn Steckplätze hat. Das ist vom Ansatz her also schon falsch dort mit festen Zahlen zu operieren, die von der Anzahl der Steckplätze abhängig sind.
Was auch auffällt ist das die beiden Listen jeweils gleiche Elemente enthalten, es also eigentlich *Zähler* sind. Die '+'- und 'o'-Elemente darin gehören schon zur Anzeige. Spiellogik und Benutzerinteraktion sollte man aber sauber trennen, damit man die Spiellogik nicht neu schreiben muss wenn man eine GUI drauf setzt.
Bei der Auswertung würde ich zwei Zahlen zurück geben, die Anzahl der richtigen Elemente und die Anzahl wie viele davon am richtigen Platz stecken. Das kann man getrennt berechnen. Und IMHO auch effizienter als Du das machst. Für die Anzahl der richtigen Elemente zählt man in Geheimnis und Tipp die ”Farben” und addiert dann jeweils den kleineren Wert zu einer Summe auf. Das wäre, eine `histogramm()`-Funktion vorausgesetzt, beides auch ziemlich kurz:
Code: Alles auswählen
anzahl_richtige = sum(
min(a, b) for a, b in zip(histogramm(geheimnis), histogramm(tipp))
)
anzahl_volltreffer = sum(a == b for a, b in zip(geheimnis, tipp))
Bei Namen die auf *einen* Ausdruck beschränkt sind, kann man auch einbuchstabige Namen verwenden, denn da sieht man gleich im Ausdruck wo die her kommen.
Du gibst zwar aus wenn man richtig geraten hat, aber das ganze geht dann trotzdem weiter‽ An der Stelle sollte man die Schleife mit ``break`` verlassen. Und dann kann man ”hinter” der Schleife auch mittels ``else`` zur Schleife das machen, was gemacht werden soll wenn der Spieler das Geheimnis *nicht* erraten hat. Damit wird der Code einfacher.
Die ganzen `str()`-Aufrufe und die ``+`` zum Verbinden von Zeichenketten und Werten sind eher BASIC als Python. Dafür gibt es die `format()`-Methode auf Zeichenketten.
Zwischenstang (komplett ungetestet):
Code: Alles auswählen
import random
def erfrage_kombination():
print('Bitte Kombination stecken!')
kombination = list()
while len(kombination) < 4:
try:
item = int(
input('Bitte Platz {} stecken:'.format(len(kombination) + 1))
)
except ValueError:
print('Keine gültige Eingabe')
else:
if 0 <= item <= 5:
kombination.append(item)
else:
print('Bitte nur Zahlen von 0 bis 5 eingeben')
print('\nEingesteckte Kombination:', kombination)
return kombination
def auswerten(geheimnis, tipp):
anzahl_volltreffer = anzahl_halbtreffer = 0
for wert in range(6):
zaehler_geheimnis = zaehler_tipp = zaehler_schwarz = 0
for geheimnis_wert, tipp_wert in zip(geheimnis, tipp):
if geheimnis_wert == tipp_wert == wert:
anzahl_volltreffer += 1
zaehler_schwarz += 1
elif geheimnis_wert == wert:
zaehler_geheimnis += 1
elif tipp_wert == wert:
zaehler_tipp += 1
tmp = zaehler_geheimnis - zaehler_schwarz
if (
zaehler_geheimnis > 0
and (
(zaehler_tipp == zaehler_geheimnis and (tmp in [1, 2]))
or (zaehler_tipp > zaehler_geheimnis and tmp == 1)
or zaehler_tipp - zaehler_schwarz == 1
)
):
anzahl_halbtreffer += 1
return (anzahl_volltreffer, anzahl_halbtreffer)
def main():
geheimnis = [random.randint(0, 5) for _ in range(4)]
for versuch_nummer in range(1, 8):
tipp = erfrage_kombination()
anzahl_volltreffer, anzahl_halbtreffer = auswerten(geheimnis, tipp)
hinweis = '+' * anzahl_volltreffer + 'o' * anzahl_halbtreffer
print('\nHinweis: {}\n'.format(hinweis))
if anzahl_volltreffer == 4:
print(
'=== Richtig===\n'
'Du hast die Lösung in {} Versuchen gefunden!'
.format(versuch_nummer)
)
break
print(
'===Falsch===\n'
'Du hast noch {} Versuch(e)\n'.format(7 - versuch_nummer)
)
else:
print(
'===Game Over===\n'
'Du hast es nicht geschafft die richtige Lösung zu finden.'
)
print('Die richtige Lösung lautet:', geheimnis)
if __name__ == '__main__':
main()
Für die GUI muss man in der Tat objektorientierte Programmierung können, und wie oben schon angedeutet sollte man die Spiellogik und die Benutzerinteraktion, also `print()` und `input()` beziehungsweise den Code für die Anzeige komplett trennen.
Was Du auch angehen solltest sind die vielen ”magischen” Zahlen im Quelltext. Also alles was mit der Anzahl der Versuche, der Anzahl der Farben, und der Anzahl der Elemente einer Kombination zu tun hat, sollte genau *einmal* als literale Zahl im Quelltext stehen, und zwar in der Regel als Definition einer Konstanten. Dann kann man das nämlich an einer Stelle ganz einfach ändern und muss nicht im ganzen Programm die gleiche Zahl anpassen und jedes mal überlegen welche Bedeutung eine Zahl hat. Und man muss ja nicht nur nach der Zahl selbst schauen, sondern eventuell auch nach der Zahl plus/minus 1 weil beispielsweise die Anzahl der Farben im `range()` als Anzahl + 1 angeben wird.