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