Zufallsteams

Stellt hier eure Projekte vor.
Internetseiten, Skripte, und alles andere bzgl. Python.
Antworten
Benutzeravatar
KEBA
User
Beiträge: 16
Registriert: Sonntag 20. April 2008, 16:53

Hallo zusammen. Ich hoffe einmal, ich habe das richtige Forum erwischt, falls nicht: Sorry...

Zum Thema: Ein Kumpel bat mit für unsere Lan-Partys ein kleines Skript zu schreiben, welches Teams zufällig erstellt. Das ganze fing mit einem simplen

Code: Alles auswählen

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

from random import shuffle

liste = range(int(raw_input("Anzahl Spieler: ")))
shuffle(liste)
mitte = len(liste)/2
print liste[:mitte], liste[mitte:]
an, hat sich dann aber zu einem größeren Projekt entwickelt. Ich bin gerade dabei Python zu lernen. Zur Zeit bin ich beim Thema OOP. Warum also nicht mal ausprobieren, wie viel man schon kann.

Nach einiger Zeit ist dann dieser Code herausgekommen.

Meine Frage ist natürlich: Was haltet ihr von dem Code? Kritik ist gerne erwünscht, immerhin hab ich das Skript geschrieben um etwas zu lernen.

Folgende Sachen gefallen mir persönlich überhaupt nicht, ich weiß allerdings nicht wie ich das ausbessern könnte:
  • OOP: Ja, das Skript besteht zum Großteil aus zwei Klassen, ich glaube aber irgendwie, dass ich das nicht wirklich im Sinne von OOP geschrieben habe:
    • Es gibt keine __init__()-Methoden, da ich keinen Sinn darin sah.
    • Die geplante Teilung in das eigentlichen Programm und der Benutzeroberfläche ist mir nicht wirklich gelungen. Ich hab aber keine Ahnung wie ich das beheben kann. Wie ließe sich z. B. setGroesse() aufteilen?
    • Mich beschleicht das Gefühl, dass ich genau diesen Code komplett ohne Klassen hätte schreiben können. Also "bringen" mir die Klassen hier gar nichts.
  • Das Setzen der x- und y-Werte in den Zeilen 42 und 43 erscheint mir sehr unschön. Die ganze Methode gefällt mir gar nicht. Leider hab ich keine bessere Idee.
  • Ich weiß nicht, ob die Endlosschleife noch in die Klasse gehört oder nicht, eine Mehtode mainloop() würde auch funktionieren...
  • Wenn man einen Namen mit einem Umlaut eingibt, wird dieser nicht korrekt dargestellt. Warum weiß ich nicht.
  • Weiterhin musste ich auf Umlaute generell verzichten, unter Windows funktionierten sie nicht.
Vielen Dank schon mal für eure Antworten! Grüße, Keba.
BlackJack

@KEBA: Das ist in der Tat nicht besonders toll. Bei Klassen sollte in der Regel eine `__init__()` existieren und in der sollten auch nach Möglichkeit alle Attribute initialisiert werden. Nach dem erstellen eines Objekts sollte es in einem "gebrauchsfertigen" Zustand sein.

Was wird mit `Team` modelliert? *Ein* Team oder *mehrere*? Wird das tatsächlich daran unterschieden ob es ein Attribut `liste` gibt? Was ein sehr nichtssagender Name ist. Namen sollten verraten was die Daten, die daran gebunden sind, bedeuten.

`Team.namen` sollte kein Klassenattribut sein, sondern an das Exemplar gebunden werden.

Das Argument `groesse` von `mische()` scheint keine einzelne Grössenangabe zu sein, sondern eine pro Team, dann sollte es aber auch `groessen` heissen.

Das Konstrukt mit `x` und `y` ist in der Tat hässlich. Auch dass an `i` etwas anderes als ganze Zahlen gebunden wird. Es wird auch nicht überprüft, ob die Rechnung aufgeht, also ob es genug Spieler gibt, oder eventuell welche übrig bleiben.

Das sich `Team.__str__()` unterschiedlich verhält, je nach dem ob nun ein Attribut `liste` existiert, oder nicht, ist unschön. Genau wie die "Indexerei" und das mit den Kommata. Dafür gibt es `str.join()`. Und wenn man zusätzlich zum Element einer Sequenz auch noch den Index braucht, sollte man `enumerate()` verwenden.

`str` ist der Name eines Grunddatentyps, den sollte man nicht für etwas anderes verwenden.

Vererbung verwendet man für "ist-ein"-Beziehungen und eine Benutzeroberfläche ist kein Team. `UI` sollte also nicht von `Team` erben.

Bei Methoden, die mit `set*` beginnen, erwartet man, dass sie ein Argument entgegennehmen und irgendein Attribut dementsprechend setzen und nicht, dass sie den Benutzer nach Eingaben fragen. Diese Methodennamen sind also irreführend. Bei `setAnzahlTeams()` wird noch nicht einmal ein Attribut gesetzt.

Man sollte Wiederholungen im Quelltext vermeiden. Bei `setNamen()` reicht es, wenn man die Eingabe *einmal* abfragt und *danach* der Liste hinzufügt. Dann muss man nur eine Endlosschleife aus dem ``while`` machen und nach der Eingabe prüfen, ob man die eventuell abbrechen muss.

Den Quelltext zum Zahlen eingeben hast Du offensichtlich nicht mit falschen Eingaben getestet. Das sind dann Endlosschleifen.

Es wird in `setGroesse()` bei der Benutzereingabe nicht geprüft ob die Eingaben insgesamt zu gross sind. Und das Zusammenrechnen lässt sich mit `sum()` *wesentlich* unkomplizierter erledigen.

Quelltext auf Modulebene sollte sich auf Definitionen von Konstanten, Funktionen, und Klassen beschränken.

`a` für die Benutzeroberfläche ist wieder so ein nichtssagender Name. Was soll das einzelne ``a`` nach dem Erstellen des `UI`-Exemplars!?

Letztlich wäre das alles mit Funktionen gegangen. Ich sehe da nicht wirklich, dass gemeinsamer innerer Zustand und Methoden, die darauf operieren, zu Objekten zusammengefasst wurden.
Benutzeravatar
jonas
User
Beiträge: 156
Registriert: Dienstag 9. September 2008, 21:03

Hi,
habe auch mal was gebastelt,
vllt. bringt es dir was und du kannst es verbessern/ausbauen:
http://paste.pocoo.org/show/106175/

MfG Jonas :wink:
DasIch
User
Beiträge: 2718
Registriert: Montag 19. Mai 2008, 04:21
Wohnort: Berlin

Wieso ist TeamGenerator eine Klasse? Was wenn die Anzahl der Teams kein Teiler von der Anzahl der Personen ist?
Benutzeravatar
str1442
User
Beiträge: 520
Registriert: Samstag 31. Mai 2008, 21:13

Das ist nen typischer Poltergeist. Sogar der Name passt :)
Benutzeravatar
Hyperion
Moderator
Beiträge: 7478
Registriert: Freitag 4. August 2006, 14:56
Wohnort: Hamburg
Kontaktdaten:

Das ganze muss dann noch damit kombiniert werden ;-)
Benutzeravatar
jonas
User
Beiträge: 156
Registriert: Dienstag 9. September 2008, 21:03

@Das Ich:

Wenn es nicht aufgeht werden die restlichen zufällig
in die Teams verteilt...

MfG Jonas :wink:
Antworten