Qualität meines CGI-Spiels? Meinungen?

Stellt hier eure Projekte vor.
Internetseiten, Skripte, und alles andere bzgl. Python.
Antworten
Chrille
User
Beiträge: 1
Registriert: Mittwoch 21. September 2005, 19:13

Ich habe ein simples CGI-Spiel (Schere, Stein, Papier) geschrieben, aus dem einfachen Grund, um sowas mal hinzubekommen. Inzwischen ist es lauffähig und anzutesten unter 84.16.230.244/cgi-bin/ssp-cgi.py
Weiter unten ist der Quelltext des Spiels zu finden bzw. unter 84.16.230.244/ssp-cgi.tar.gz oder 84.16.230.244/ssp-cgi.zip könnt ihr ihn herunterladen.

Mich interessiert eure Meinung:
Habt ihr irgendwelche Stilblüten oder Fehler entdeckt oder fallen euch Anregungen ein?
Ich werde vielleicht später einmal noch versuchen eine Login-Funktion und, darauf aufbauend, Top10-Listen (wer hat die größte Sieges- / Verluststrähne, etc.) einzubauen.
Ich habe auch schon etwas passendes gefunden: Die LoginTools scheinen genau das richtige für mein Vorhaben zu sein. Leider schlug das importieren fehl, als ich versucht habe, die LoginTools testweise einzubinden. Nunja, das hat ja noch Zeit. :)

Hier nun der Quelltext:

Code: Alles auswählen

#!/usr/bin/env python
# -*- coding: UTF-8 -*-

import cgi
import random

radio = cgi.FieldStorage()
answer_dict = {1: "Schere", 2: "Stein", 3: "Papier"}

if radio.has_key("ssp"):
    radio_choice = radio["ssp"].value
else:
    radio_choice = None

def checked(ui):
    if ui == None:
        return ("%s", 'checked="checked"', "", "")
    elif ui == "Stein":
        return ("%s", "", 'checked="checked"', "")
    elif ui == "Papier":
        return ("%s", "", "", 'checked="checked"')
    else:
        return ("%s", 'checked="checked"', "", "")

result_page = """Content-type: text/html\n
<html>
   <head><meta http-equiv="Content-Type" content="text/html; charset=UTF-8"/><title>Schere, Stein, Papier</title></head>
   <body>
      <h1>Schere, Stein, Papier</h1>
      <h2>%s</h2>
      <hr>
      <h2>Neues Spiel?</h2>
      <form method="post" action="/cgi-bin/ssp-cgi.py">

         <p>Schere, Stein oder Papier?<br />
         <input type="radio" name="ssp" value="Schere" %s> Schere<br>
         <input type="radio" name="ssp" value="Stein" %s> Stein<br>
         <input type="radio" name="ssp" value="Papier" %s> Papier<br>
         <input type="submit" value="Go!">
         </p>
      </form>
   </body>
</html>\n""" % checked(radio_choice)

if not radio.has_key("ssp"):
   print "Location: /ssp_index.html\n"
   
else:
    # computer's choice and player's choice
    cc = answer_dict[random.randint(1,3)]
    pc = radio["ssp"].value
    
    if cc == pc:
        print result_page % "Unentschieden!"
    elif cc == "Schere" and pc == "Stein":
        print result_page % "Gewonnen! Stein schlägt Schere!"
    elif cc == "Schere" and pc == "Papier":
        print result_page % "Verloren! Schere schlägt Papier!"
    elif cc == "Stein" and pc == "Schere":
        print result_page % "Verloren! Stein schlägt Schere!"
    elif cc == "Stein" and pc == "Papier":
        print result_page % "Gewonnen! Papier schlägt Schere!"
    elif cc == "Papier" and pc == "Schere":
        print result_page % "Gewonnen! Schere schlägt Papier!"
    else:
        print result_page % "Verloren! Papier schlägt Stein!"
Quelltext der ssp_index.html:

Code: Alles auswählen

<html>
   <head><title>Schere, Stein, Papier</title></head>
   <body>
      <h1>Schere, Stein, Papier</h1>

      <form method="post" action="/cgi-bin/ssp-cgi.py">

         <p>Schere, Stein oder Papier?<br />
         <input type="radio" name="ssp" value="Schere" checked="checked"> Schere<br>
         <input type="radio" name="ssp" value="Stein"> Stein<br>
         <input type="radio" name="ssp" value="Papier"> Papier<br>
         <input type="submit" value="Go!">
         </p>
      </form>
   </body>
</html>
Leonidas
Python-Forum Veteran
Beiträge: 16025
Registriert: Freitag 20. Juni 2003, 16:30
Kontaktdaten:

Ich würde es noch in Funktionen verpacken, aber sonst ist es ganz in Ordnung.
My god, it's full of CARs! | Leonidasvoice vs (former) Modvoice
Benutzeravatar
jens
Python-Forum Veteran
Beiträge: 8502
Registriert: Dienstag 10. August 2004, 09:40
Wohnort: duisburg
Kontaktdaten:

Ich hab mal ein PyLucid Plugin draus gemacht. Könnt ihr hier in Aktion sehen:
http://www.jensdiemer.de/Programmieren/ ... ere+Papier

Der Sourcecode:

Code: Alles auswählen

#!/usr/bin/env python
# -*- coding: UTF-8 -*-

#~ import cgi
import random

#~ radio = cgi.FieldStorage()

class SteinScherePapier:

    #_______________________________________________________________________
    # Module-Manager Daten

    module_manager_data = {
        #~ "debug" : True,
        "debug" : False,

        "lucidTag" : {
            "must_login"    : False,
            "must_admin"    : False,
            "CGI_dependent_actions" : {
                "result"   : {
                    "CGI_must_have"      : ("ssp",)
                },
            }
        }
    }

    #_______________________________________________________________________

    def __init__(self, PyLucid):
        self.CGIdata    = PyLucid["CGIdata"]
        self.page_msg   = PyLucid["page_msg"]

    def lucidTag(self):
        print '<form method="post" action="%s">' % self.main_action_url
        print '  <p>Schere, Stein oder Papier?<br />'
        print '  <input type="radio" name="ssp" value="Schere"> Schere<br>'
        print '  <input type="radio" name="ssp" value="Stein"> Stein<br>'
        print '  <input type="radio" name="ssp" value="Papier"> Papier<br>'
        print '  <input type="submit" value="Go!">'
        print '  </p>'
        print '</form>'

    def result(self):
        # computer's choice and player's choice
        answer_dict = {1: "Schere", 2: "Stein", 3: "Papier"}
        cc = answer_dict[random.randint(1,3)]
        pc = self.CGIdata["ssp"]

        if cc == pc:
            self.page_msg("Unentschieden!")
        elif cc == "Schere" and pc == "Stein":
            self.page_msg("Gewonnen! Stein schlägt Schere!")
        elif cc == "Schere" and pc == "Papier":
            self.page_msg("Verloren! Schere schlägt Papier!")
        elif cc == "Stein" and pc == "Schere":
            self.page_msg("Verloren! Stein schlägt Schere!")
        elif cc == "Stein" and pc == "Papier":
            self.page_msg("Gewonnen! Papier schlägt Schere!")
        elif cc == "Papier" and pc == "Schere":
            self.page_msg("Gewonnen! Schere schlägt Papier!")
        else:
            self.page_msg("Verloren! Papier schlägt Stein!")

        print "<h2>Neues Spiel?</h2>"

        # form wieder anzeigen
        self.lucidTag()
        return

GitHub | Open HUB | Xing | Linked in
Bitcoins to: 1JEgSQepxGjdprNedC9tXQWLpS424AL8cd
ProgChild
User
Beiträge: 210
Registriert: Samstag 9. April 2005, 10:58
Kontaktdaten:

In deinem HTML fehlt noch der Doctype. Ansonsten siehts gut aus :D
BlackJack

Chrille hat geschrieben: Mich interessiert eure Meinung:
Habt ihr irgendwelche Stilblüten oder Fehler entdeckt oder fallen euch Anregungen ein?

[...]

Hier nun der Quelltext:

Code: Alles auswählen

#!/usr/bin/env python
# -*- coding: UTF-8 -*-

import cgi
import random

radio = cgi.FieldStorage()
answer_dict = {1: "Schere", 2: "Stein", 3: "Papier"}
Eine einfache Liste hätte es hier auch getan:

Code: Alles auswählen

answers = ['Schere', 'Stein', 'Papier']

Code: Alles auswählen

if radio.has_key("ssp"):
    radio_choice = radio["ssp"].value
else:
    radio_choice = None

def checked(ui):
    if ui == None:
        return ("%s", 'checked="checked"', "", "")
    elif ui == "Stein":
        return ("%s", "", 'checked="checked"', "")
    elif ui == "Papier":
        return ("%s", "", "", 'checked="checked"')
    else:
        return ("%s", 'checked="checked"', "", "")
Wenn man bei einem Rückgabewert einen Teil hat, der konstant ist, dann hängt der nicht von der Funktion ab und gehört auch nicht dort hinein. Das betrifft den ersten Wert im Tupel.

Es ist auch meistens eine gute Idee Redundanz zu vermeiden, also möglichst wenig identische Informationen zu wiederholen. Das betrifft in diesem Spiel die Namen der Gegenstände. Die Zeichenketten 'Schere', 'Stein' und 'Papier' sollten im Idealfall nur einmal im Programm vorkommen, weil man damit die Wahrscheinlichkeit von Tippfehlern verringert und Änderungen einfacher macht, da die Daten nur an einer Stelle geändert werden müssen und nicht über den gesamten Quelltext verteilt sind.

Die Funktion `checked()` könnte man zum Beispiel so schreiben, dass sie die Position der Zeichenkette in der `answers` Liste ermittelt und entsprechend die 'checked="checked"' Zeichenkette an die passende Stelle des Rückgabewertes setzt.

Code: Alles auswählen

def checked(ui):
    result = [''] * len(answers)
    try:
        result[answers.index(ui)] = 'checked="checked"'
    except ValueError:
        pass
    return tuple(result)
Falls der Wert von `ui` nicht in den Antworten enthalten ist, dann ist kein Gegenstand ausgewählt.

Code: Alles auswählen

result_page = """Content-type: text/html\n
<html>
   <head><meta http-equiv="Content-Type" content="text/html; charset=UTF-8"/>
   <title>Schere, Stein, Papier</title></head>
   <body>
      <h1>Schere, Stein, Papier</h1>
      <h2>%s</h2>
      <hr>
      <h2>Neues Spiel?</h2>
      <form method="post" action="/cgi-bin/ssp-cgi.py">

         <p>Schere, Stein oder Papier?<br />
         <input type="radio" name="ssp" value="Schere" %s> Schere<br>
         <input type="radio" name="ssp" value="Stein" %s> Stein<br>
         <input type="radio" name="ssp" value="Papier" %s> Papier<br>
         <input type="submit" value="Go!">
         </p>
      </form>
   </body>
</html>\n""" % checked(radio_choice)
Hier müsstest Du das erste '%s' in der HTML Zeichenkette durch '%%s' ersetzen um zu verhindern, dass das Prozentzeichen als Platzhalter erkannt wird.

Schon wieder Redundanz: Der Teil mit den <input> Tags könnte mit der Liste der möglichen Antworten durch eine Funktion generiert werden. Das liesse sich auch mit der Funktionalität der `checked()` Funktion kombinieren:

Code: Alles auswählen

def make_inputs(ui):
    template = '<input type="radio" name="ssp" value="%s" %s> %s<br />'
    result = list()
    for obj in answers:
        if ui == obj:
            checked_str = 'checked="checked"'
        else:
            checked_str = ''
        result.append(template % (obj, checked_str, obj))
    return '\n'.join(result)

Code: Alles auswählen

if not radio.has_key("ssp"):
   print "Location: /ssp_index.html\n"
   
else:
    # computer's choice and player's choice
    cc = answer_dict[random.randint(1,3)]
    pc = radio["ssp"].value
Hier fragst Du eine Information ab, die weiter oben schon einmal abgefragt wurde und unter dem Namen `radio_choice` verfügbar ist.

Die Auswahl des Computers kann man wie folgt ermitteln, wenn die Antworten in einer Liste stehen:

Code: Alles auswählen

cc = random.choice(answers)

Code: Alles auswählen

    if cc == pc:
        print result_page % "Unentschieden!"
    elif cc == "Schere" and pc == "Stein":
        print result_page % "Gewonnen! Stein schlägt Schere!"
    elif cc == "Schere" and pc == "Papier":
        print result_page % "Verloren! Schere schlägt Papier!"
    elif cc == "Stein" and pc == "Schere":
        print result_page % "Verloren! Stein schlägt Schere!"
    elif cc == "Stein" and pc == "Papier":
        print result_page % "Gewonnen! Papier schlägt Schere!"
    elif cc == "Papier" and pc == "Schere":
        print result_page % "Gewonnen! Schere schlägt Papier!"
    else:
        print result_page % "Verloren! Papier schlägt Stein!"
Hier gilt wieder das oben erwähnte Vermeiden von Informationswiederholung. Die Zeichenketten 'Schere', 'Stein' und 'Papier' werden IMHO viel zu oft hingeschrieben. Ausserdem sind die vielen ``elif`` Zweige ein wenig unübersichtlich und viel Tipparbeit. Übersichtlicher wäre es, die Regeln mit einer passenden Datenstruktur auszudrücken und eine Funktion zu schreiben, die Anhand der Regeln und der gegebenen Werte entscheidet wer gewonnen hat.

Die einfachste Datenstruktur wäre in diesem Fall wohl eine Liste, weil die Regeln als Graph dargestellt einen einfachen Kreis bilden. Schere schlägt Papier, Papier schlägt Stein und Stein schlägt Schere. Wenn man die Gegenstände also in dieser Reihenfolge in einer Liste speichert, dann kann man daran schon ablesen wer gewonnen hat:

Code: Alles auswählen

answers = ['Schere', 'Papier', 'Stein']

def is_player_winner(computer_choice, player_choice):
    if computer_choice == player_choice:
        return None
    else:
        computer_index, player_index = map(answers.index, (computer_choice,
                                                           player_choice))
        return (player_index + 1) % len(answers) == computer_index

winner = is_player_winner(cc, pc)
print { None: 'Unentschieden', True: 'Gewonnen', False: 'Verloren' }[winner]
Quelltext der ssp_index.html:

Code: Alles auswählen

<html>
   <head><title>Schere, Stein, Papier</title></head>
   <body>
      <h1>Schere, Stein, Papier</h1>

      <form method="post" action="/cgi-bin/ssp-cgi.py">

         <p>Schere, Stein oder Papier?<br />
         <input type="radio" name="ssp" value="Schere" checked="checked"> Schere<br>
         <input type="radio" name="ssp" value="Stein"> Stein<br>
         <input type="radio" name="ssp" value="Papier"> Papier<br>
         <input type="submit" value="Go!">
         </p>
      </form>
   </body>
</html>
Und zum Schluss nochmal unnötige Redundanz. Hier steht fast das gleiche HTML wie im Programm. Das lässt sich vermeiden wenn man das Programm so umschreibt, dass es auch den Fall berücksichtigt, dass kein Gegenstand ausgewählt wurde und dann davon ausgeht, dass noch nicht gespielt wurde. So kann man sich eine externe HTML Seite sparen.
Benutzeravatar
gerold
Python-Forum Veteran
Beiträge: 5555
Registriert: Samstag 28. Februar 2004, 22:04
Wohnort: Oberhofen im Inntal (Tirol)
Kontaktdaten:

BlackJack hat geschrieben:

Code: Alles auswählen

answers = ['Schere', 'Papier', 'Stein']

def is_player_winner(computer_choice, player_choice):
    if computer_choice == player_choice:
        return None
    else:
        computer_index, player_index = map(answers.index, (computer_choice,
                                                           player_choice))
        return (player_index + 1) % len(answers) == computer_index

winner = is_player_winner(cc, pc)
print { None: 'Unentschieden', True: 'Gewonnen', False: 'Verloren' }[winner]
Hi BlackJack!

Warum soll man einfache Dinge nicht auf eine einfache Art erledigen?

Ich denke nicht, dass dieser Codeabschnitt das Programm vereinfacht.
Den Originalcode konnte man mit den Augen überfliegen und wusste sofort was darin passiert. Deine Funktion "is_player_winner" kann nicht vom Auge überflogen werden. Diesen Code muss man von Anfang bis Ende lesen um zu wissen was darin passiert. Das empfinde ich nicht als Vorteil, auch wenn dein Code lehrreich ist und Möglichkeiten von Python aufdeckt.

lg
Gerold
:-)
http://halvar.at | Kleiner Bascom AVR Kurs
Wissen hat eine wunderbare Eigenschaft: Es verdoppelt sich, wenn man es teilt.
BlackJack

gerold hat geschrieben: Warum soll man einfache Dinge nicht auf eine einfache Art erledigen?

Ich denke nicht, dass dieser Codeabschnitt das Programm vereinfacht.
Den Originalcode konnte man mit den Augen überfliegen und wusste sofort was darin passiert.
Gerade das fand ich nicht. Da gab's einen Haufen ``elif`` mit jeweils zwei Bedingungen. Ich könnte nicht so schnell erfassen ob sich irgendwo ein Tippfehler eingeschlichen hat bei den Abfragen, oder ob alle möglichen Kombinationen geprüft wurden und ob das richtige Ergebnis bei jeder Prüfung heraus kommt.

Wenn man das Prinzip mit der "Ringliste" per Modulo-Operation verstanden hat, dann kann man IMHO viel eher sagen, dass der Code korrekt ist weil man ausser der trivialen "Unentschieden"-Abfrage nur noch einen anderen Fall betrachten muss.
Deine Funktion "is_player_winner" kann nicht vom Auge überflogen werden. Diesen Code muss man von Anfang bis Ende lesen um zu wissen was darin passiert.
Du kannst den ursprünglichen Code leicht überfliegen und denken Du weisst was da passiert. Wenn Du's genau wissen willst musst Du jeden `elif`-Zweig lesen, verstehen und Dir merken welche Fälle schon behandelt wurden.

Und man hat dort mehrmals die Zeichenketten für 'Schere', 'Stein' und 'Papier' zu stehen. Wenn man zum Beispiel die Schere durch einen Brunnen ersetzen will (eine Variante des Spiels), dann muss man aufpassen, dass man das überall im Quelltext macht.

Das mag bei diesem kleinen Programm nicht so schwer sein, aber der OP hat auch nach (Programmier)Stil gefragt.
Antworten