diff --git a/Bewertung_Siedler.md b/Bewertung_Siedler.md new file mode 100644 index 0000000..93faced --- /dev/null +++ b/Bewertung_Siedler.md @@ -0,0 +1,134 @@ +# Bewertung Catan +## 🥇 27.5 / 17.5 Punkte von 30 + +👫 brandleo@students.zhaw.ch; fassband@students.zhaw.ch; schrom01@students.zhaw.ch; zieglmic@students.zhaw.ch; amadoste@students.zhaw.ch + +## Zusammenarbeit + +* Durch untenstehende Probleme verursacht durch die Herren Ziegler und Amador werden pro Person 5 Punkte abgezogen (Total **10 Punkte**) + * Wiederholtes nicht Einhalten von Versprechen und Terminen + * Unverhältnismässiger Beitrag an produktivem Code + * Erinnerung: + * 4 ECTS Credits entsprechen 120 Arbeitsstunden + * Bei 12 Wochen bis zur letzten Abgabe sind das 10 Stunden pro Woche + * Bei zu wenig Zeit oder auch inhaltlicher Überforderung können Sie mögliche Optionen mit Olaf Stern (strf) und Patrick Feisthammel (fame) +* Ausnahme: + * Die Herren Brandenberger, Fassbind und Schenk erhalten **keinen** Abzug, da Sie alles versucht haben die Zusammenarbeit aufrecht und das Team intakt zu erhalten + +## Grundfunktionalität + +👨‍🏫 15.5 Punkte von 18 + +### Funktionalität (1.5 Punkte Abzug) + +> Sie haben die Funktionalität wie gefordert umgesetzt. + +* Strassen bauen: + * Können in das Wasser gehen + * Können durch gegnerische Siedlungen gebaut werden +* Würfeln + * Bei einem Wurf von 7 werden die halbierten Ressourcen aufgerundet, sollten aber abgerundet werden (bei 9 müssen 4 abgegeben werden) + * Problem: `resourceArrayList.size() - (resourceArrayList.size() / 2)` -> `resourceArrayList.size() / 2` + + * `throwDice` sollte nur die Ressourcen zurückgeben, welche effektiv verteilt worden sind + +* Die Initialen Siedlungen und Strassen werden nicht vom Pool abgezogen + +### Clean Code und Konstrukte (0.5 Punkt Abzug) + +> * Sie halten die Vorgaben hinsichtlich einsetzbarer Konstrukte ein. +> * Sie halten die Vorgaben hinsichtlich Clean Code ein. + +* `SiedlerGameTest` + * Funktionsnamen sollten mit einem Kleinbuchstaben beginnen + * `new ArrayList<>(List.of(/* ... */))` kann direkt `List.of(/* ... */)` sein + +### Klassendesign + +> * Sie haben eine sinnvolle Aufteilung in Klassen und Klassendefinitionen gefunden. +> * Das Klassendiagramm zeigt die Aufteilung verständlich. + +* Gute Klassenaufteilung +* Übersichtliches Diagramm + +### Tests (0.5 Punkte Abzug) + +> Sie haben sinnvolle Test Cases für die Klasse `SiedlerGame` definiert und diese erfolgreich ausgeführt. + +* Es sollten komplette Collections mit equals verglichen werden und nicht nur die einzelnen Elemente: +```java +Assertions.assertEquals(0, resources.get(Config.Resource.BRICK)); +Assertions.assertEquals(0, resources.get(Config.Resource.GRAIN)); +Assertions.assertEquals(0, resources.get(Config.Resource.LUMBER)); +Assertions.assertEquals(0, resources.get(Config.Resource.ORE)); +Assertions.assertEquals(0, resources.get(Config.Resource.WOOL)); +// should be: +Assertions.assertEquals(Map.of( + Config.Resource.BRICK, 0, + Config.Resource.GRAIN, 0, + Config.Resource.LUMBER, 0, + Config.Resource.ORE, 0, + Config.Resource.WOOL, 0 +), resources); +``` +* In `startSiedlerGameWithLowerThanMinimumWinPoints` hat es kein assert + +### Dokumentation + +> Ihr Code ist in `JavaDoc` sauber dokumentiert. + +* Sehr gute Dokumentation +* Besonders die exakten Limitationen und Fehlerfälle + +## Erweiterung «Städte» + +👨‍🏫 5 Punkte von 5 + +> Die Erweiterung «Städte» ist mit Vererbung umgesetzt. + +* Funktioniert wie gewünscht + +## Erweiterung «Längste Strasse» oder «Räuber» + +👨‍🏫 7 Punkte von 7 + +* «Längste Strasse» wurde umgesetzt +* Funktioniert wie gewünscht + +## Zusätzliche Hinweise (nicht bewertet) + +* Wenn `instanceof` verwendet wird, deutet das auf suboptimale Vererbung hin + * Folgendes könnte mit einer `getWinPoints` Funktion in den Structures gelöst werden + * Vorteil: Wenn eine neue Structure dazu kommt, muss nicht anderer Code angepasst werden + * Bei reinen Datenklassen lohnt es sich zu überlegen: Braucht es sie überhaupt? Können sie noch Funktionen brauchen? +```java +int newWinPoints = 0; +if (structure instanceof City) { + newWinPoints = 2; +} else if (structure instanceof Settlement) { + newWinPoints = 1; +} +``` +* Der Return-Wert von `SielderGame.subtractResourceFromPlayer` wird nie verwendet +* Die Instanz von `Random` kann als `static final` definiert werden + * Weniger Objekte müssen erzeugt werden + * Vereinfacht das Reproduzieren von Abläufen mit Hilfe von einem Seed +* In `Siedler.gamePhase` sollte `parser.quit()` nach dem Loop ausgeführt werden + * Dies wurde vergessen, wenn der Player gewonnen hat +* Vereinfachung: + +```java +boolean successful = false; +do { + if (game.placeInitialSettlement(parser.getPoint(), payout)) { + successful = true; + } else { + parser.errorMessage(); + } +} while (!successful); +// can be written as +while (!game.placeInitialSettlement(parser.getPoint(), payout)) { + parser.errorMessage(); +} +``` +