Zahlenraten - Anfänger Snippet

Code-Stücke können hier veröffentlicht werden.
Antworten
Benutzeravatar
xpilz
User
Beiträge: 76
Registriert: Sonntag 11. April 2010, 12:46
Wohnort: Deutschland
Kontaktdaten:

Hallo :D, ich wollte euch mal ein snippet von mir zeigen und wollte fragen ob es irgendwo Sachen gibt die ich besser machen sollte / in Zukunft beachten sollte und ob mein Schreibstil so lesbar ist.

Code: Alles auswählen

from random import randint

def guessNumber(lvl):
    tN=0
    gN=0
    tries=0
    if lvl == 1:
        tN = randint(1,10)
    elif lvl == 2:
        tN = randint(1,25)
    elif lvl == 3:
        tN = randint(1,50)
    while gN != tN:
        print('//////////////////////////')
        print('Du hast noch ',3-tries,' Versuch/e.')
        gN=int(input('Rate die Zahl:'))
        if gN == tN:
            win()
        else:
            print('Diese Zahl war falsch.')
            if tries == 3:
                loose(tN)
            tries = tries +1
            if gN < tN:
                print('Versuche es mit einem hoeherem Wert.')
            if gN > tN:
                print('Versuche es mit einem geringerem Wert.')             
            
            
def start():
    print('//////////////////////////')
    print('Bitte Geben sie einen Schwierigkeitsgrad ein.\n1 - Leicht (1-10)\n2 - Mittel(1-25)\n3 - Schwer(1-50)')
    x = int(input('Waehle Schwierigkeitsgrad:'))
    if type(x) == int:
        guessNumber(x)
        print('//////////////////////////')
    else:
        print('Falscher Wert.')
        start()

def loose(tN):
    print('Schade, du hast verloren.')
    print('Die Zahl lautete: %s' %tN)
    print('//////////////////////////')
    x = input('Willst du nochmal spielen? (J/N):')
    x=x.lower()
    if x == 'j':
        start()
    elif x == 'n':
        quit()
    else:
        print('Falsche Eingabe.\nSpiel wird neu gestartet.')
        start()

def win():
    print('Super, du hast gewonnen.')
    print('//////////////////////////')
    x = input('Willst du nochmal spielen? (J/N):')
    x=x.lower()
    if x == 'j':
        start()
    elif x == 'n':
        quit()
    else:
        print('Falsche Eingabe.\nSpiel wird neu gestartet.')
        start()

start()
Vielen Dank für Kritik im Vorraus.
Benutzeravatar
jonas
User
Beiträge: 156
Registriert: Dienstag 9. September 2008, 21:03

Was mir ganz auf die Schnelle einfällt:

1) tN, gN -> aussagekräftigere Namen verwenden
2) kein input() sondern raw_input() verwenden (falls Python < 3.0)
3) if __name__ == '__main__': fehlt (wird benötigt, damit das Modul, falls es importiert wird, nicht sofort ausgeführt wird)
4) PEP8

Lg, Jonas
Benutzeravatar
gkuhl
User
Beiträge: 600
Registriert: Dienstag 25. November 2008, 18:03
Wohnort: Hong Kong

Die Funktion ``start()`` solltest du überarbeiten. Die Typüberprüfung ist unnötig, weil bereits eine ValueError entsteht, wenn du versuchst die Eingabe umzuwandeln. Verwende stattdessen ``try`` und ``except``. Weiter erlaubst du Zahlen größer 3, obwohl du nur drei Level hast. Du kannst am besten den Benutzer direkt die obere Grenze (max_value) des Intervals wählen lassen und dann randint(1,max_value) aufrufen.
EyDu
User
Beiträge: 4881
Registriert: Donnerstag 20. Juli 2006, 23:06
Wohnort: Berlin

Zeilen 7 bis 12 kann man prima zusammenfassen.

Die wirren Funktionsaufrufe von win, start und loose sind sehr unschön. Du solltest immer an eine Stelle zurückkommen, welche den Status verwaltet.

In Zeile 26 tut es auch ein else.

Die ganzen "////////////" kann man durch "/"*10 ersetzen oder am besten gleich in eine eigene Funktion auslagern.

Zeile 33 & 34 sind natürlich grober Unfug.

Die ja/nein-Abfragen kann man einmal als Funktion schreiben und nicht doppelt.

Und dann natürlich die ganzen magic numbers, welche besser Konstanten sein sollten.
Das Leben ist wie ein Tennisball.
BlackJack

@xpilz: Fuktionsaufrufe sind keine Sprungbefehle wie GOTO in BASIC. Du kannst den Programmfluss nicht so umsetzen ohne das völlig unnötig immer mehr Speicher für Objekte in den lokalen Namensräumen verbraucht wird und ausserdem ist bei CPython damit nach normalerweise maximal 1000 solcher Aufrufe Schluss und das Programm bricht mit einer Fehlermeldung ab.

Variablen muss man in Python nicht am Anfang deklarieren `tN` und `gN` müssen am Anfang von `guessNumber()` also nicht auf 0 gesetzt werden. Und auch bei `tries` würde ich das anders machen. Du weisst ja wie oft die Schleife durchlaufen wird. Und wenn man das vor einer Schleife weiss, dann nimmt man keine ``while``- sondern eine ``for``-Schleife. Die Bedingung bei Deiner ``while``-Schleife ist auch überflüssig die wird nämlich nie unwahr. In dem Fall wo `gN` gleich `tN` ist, verlässt Du die Schleife ja bevor sie noch einmal durchlaufen kann und die Bedingung wahr werden würde.

Bei robusten Programmen sollte man bis zu einem gewissen Grad auch mit falschen Eingaben klar kommen. Was passiert denn, wenn man `guessNumber()` mit einem anderen `lvl` als 1, 2, oder 3 aufruft? Und auch `lvl` ist ein schlechter Name. Das heisst `level`. Am tippen der zwei fehlenden 'e's wirst Du schon nicht sterben. ;-) In den drei Zweigen unterscheidet sich ja eigentlich auch nur eine Zahl also ein Datum und kein Code. So etwas kann man in Datenstrukturen auslagern. Ungetestet:

Code: Alles auswählen

    upper_limits = [10, 25, 50]
    if not (1 <= level < len(upper_limits)):
        raise ValueError('level not in 1-%d' % len(upper_limits))
    number = randint(1, upper_limits[level - 1])
In `start()` wird nie der ``else``-Zweig ausgeführt. Das Objekt das an `x` gebunden ist nach der Zuweisung zwingend vom Typ `int`. (Also mal den pathologischen Fall aussen vor gelassen, dass irgendwo jemand den Namen `int` an was anderes gebunden hat und "global" gemacht hat.)
Benutzeravatar
xpilz
User
Beiträge: 76
Registriert: Sonntag 11. April 2010, 12:46
Wohnort: Deutschland
Kontaktdaten:

@jonas: Danke, für die schnelle Antwort.
1) tn und gN = true number und guessed number.. ich weiß nicht.. Irgendwie hab ich es mir angewöhnt die Variablen Namen so klein zu halten wie es geht.
2) Python version ist größer als 3. Erkennt man das nicht an print(..)? Keine Ahnung :roll:.
3) Das werde ich ändern, danke + Beschreibung. Hab ich mich auch gefragt.
4) Ja, es wäre besser, mir das mal durchzulesen.

@gkuhl: Typüberprüfung, try und except werde ich reinmachen, aber was bringt das? Bessere Lesbarkeit, oder was?
max_value, nette Idee danke gkuhl.

@EyDu: Zeilen 7-12: Ok, ich werde dann die Idee von gkuhl nehmen
Du kannst am besten den Benutzer direkt die obere Grenze (max_value) des Intervals wählen lassen und dann randint(1,max_value) aufrufen.
Wirre Funktionsaufrufe: Ok, ich werde mir mal ein paar Beispiele anschauen und es dann versuchen hierauf anzuwenden.
Else in Zeile 26: Danke, das werde ich mir merken.
"/"*10: Dito. Nur tut es was zu Sache ob man Strings mit ' oder mit " eingrenzt?
33 & 34 Unfug: Ja, das werde ich dann auch mit gkuhl's Beispiel lösen.
Ja/nein: Guter Tipp. Danke.
Magic Numbers: :?: Ich werde mich da mal darüber informieren..

@BlackJack: Gut diese goto Schreibweise habe ich mir wahrscheinlich von C++ und/oder Batch angeeignet. Die überflüssigen Variablen werde ich löschen. Die Sache mit der Schleife, ist eine gute Idee, das werde ich dann auch in Zukunft beachten. Der Code sieht für mich sehr Fortgeschritten aus, brauchte ein wenig länger um ihn zu verstehen.. Trotzdem er gefällt mir ich werde ihn auf meinem Blog veröffentlichen wenn du nichts dagegen hast :). Gut werde ich auch entfernen. Sollte ich den nächsten versuch mit paste.pocoo posten, weil der Code ist ja schon überdurschnittsmäßig lang für ein Snippet. Oder nicht?
Zuletzt geändert von xpilz am Donnerstag 15. April 2010, 15:35, insgesamt 1-mal geändert.
Benutzeravatar
gkuhl
User
Beiträge: 600
Registriert: Dienstag 25. November 2008, 18:03
Wohnort: Hong Kong

xpilz hat geschrieben:Typüberprüfung, try und except werde ich reinmachen, aber was bringt das? Bessere Lesbarkeit, oder was?
Weil es, im Gegensatz zu deiner Lösung, funktioniert. Gebe einfach mal '4.2' oder 'keine Zahl' (beides ohne Anführungszeichen) ein.
BlackJack

@xpilz: Den Programmfluss auf diese Weise mit Funktionsaufrufen zu gestalten ist auch unter C++ genau so eine schlechte Idee. Auch dort wird der Stack mit den ganzen lokalen Variablen "belastet" und wächst immer weiter an. Clevere Compiler können vielleicht hier und da "tail call optimization" anwenden und das Problem eindämmen, aber der Sprachstandard schreibt TCO nicht vor, damit darf man also nicht rechnen.
Benutzeravatar
xpilz
User
Beiträge: 76
Registriert: Sonntag 11. April 2010, 12:46
Wohnort: Deutschland
Kontaktdaten:

Super, es funktioniert jetzt sogar mit dem überarbeitetem Code :D.
Wer es sich nochmal anschauen mag:
Zahlenraten
Oder hab ich irgendetwas vergessen oder habt ihr noch Verbesserungsvorschläge? Ansonsten vielen Dank für die Tips.
BlackJack

Naja es ist immer noch der Programmfluss über endlose Funktionsaufrufe die nie zurückkehren drin.
Benutzeravatar
xpilz
User
Beiträge: 76
Registriert: Sonntag 11. April 2010, 12:46
Wohnort: Deutschland
Kontaktdaten:

Kann man das mit return verhindern? Wenn ja, ist None als return Wert akzeptabel? Und wenn nein wie dann?
Benutzeravatar
jbs
User
Beiträge: 953
Registriert: Mittwoch 24. Juni 2009, 13:13
Wohnort: Postdam

Vielleicht hilft dir das:

Code: Alles auswählen

from random import randrange
from itertools import count

def ask(question, callback=lambda x: x, errormessage='Falsche Eingabe'):
    while True:
        try:
            return callback(raw_input(question))
        except ValueError:
            print errormessage

def valid_yesno(s):
    s = s.strip().lower()
    if s in ('j','n'):
        return s
    raise ValueError

def cli():
    ask_limit = 'Geben Sie ein Limit ein: '
    ask_guess = 'Geben Sie eine Zahl ein: '
    ask_newgame = 'Erneut spielen (j/n)'
    
    limit = ask(ask_limit, int)
    while True:
        number = randrange(1, limit+1)
        for round in count(1):
            print 'Runde %s' %round
            guess = ask(ask_guess, int)
            
            if guess == number:
                print 'Richtig!'
                print 'Geloest in %s Runden' %round
                break
            elif guess < number:
                print 'Ihre Nummer ist zu klein'
            else:
                print 'Ihre Nummer ist zu gross'
        
        if ask(ask_newgame, valid_yesno) == 'n':
            break
            
if __name__ == '__main__':
    cli()
[url=http://wiki.python-forum.de/PEP%208%20%28%C3%9Cbersetzung%29]PEP 8[/url] - Quak!
[url=http://tutorial.pocoo.org/index.html]Tutorial in Deutsch[/url]
sma
User
Beiträge: 3018
Registriert: Montag 19. November 2007, 19:57
Wohnort: Kiel

BlackJack hat geschrieben:Naja es ist immer noch der Programmfluss über endlose Funktionsaufrufe die nie zurückkehren drin.
Da die aber alle alle endrekursiv sind, wäre es nicht schlimm, wenn Python das optimieren könnte. In einer funktionalen Programmiersprache (Scheme, Erlang, Haskell, Ocaml, ...) wäre das so sogar guter Stil :)

Stefan
Benutzeravatar
xpilz
User
Beiträge: 76
Registriert: Sonntag 11. April 2010, 12:46
Wohnort: Deutschland
Kontaktdaten:

In einer funktionalen Programmiersprache (Scheme, Erlang, Haskell, Ocaml, ...) wäre das so sogar guter Stil :)
Na dass ist doch mal schön zu hören. Aber da es mir hier um Python geht, werde ich mir diesen Stil wohl abgewöhnen müssen.. Ich werde in nächster Zeit mal ein paar Ideen aus dem Code von jbs nehmen und den nochmal hier hereinposten. Ich hoffe das sieht dann nicht so aus als hätte ich alles übernommen..
Antworten