gruppe06-hufflepuff-projekt3-catan/Bewertung_Siedler.md

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:

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?

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:

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();
}