Pfadfinder Regel

Ich habe schon vor einigen Posts angedeutet, dass ich noch etwas über den BlueMapSignMarkers Code einige Worte verlieren will.

Ich dachte ich würde jetzt noch warten, bis mein PR bei KaiijuMC durch ist. Da das Projekt aber inaktiv ist, werde ich mal meine Gedanken dazu äussern.

Update Libs

Also die Libs wieder auf den richtigen Stand bringen, war die kleinste Änderung. Dazu hab ich einfach die aktuellen Dokus der Libs studiert, wie diese einzubinden sind.

https://github.com/BlueMap-Minecraft/BlueMapAPI/wiki https://docs.papermc.io/paper/dev/project-setup

Bei Paper-API hat sich das Repository geändert, ohne Doku Studium hätte ich da wohl geärgert, weshalb ich die neuen Packages nicht finde.

Code Cleanup

Als nächstes wollte ich den Code einfach aufräumen. Dazu sprang mir folgende Felder sofort ins Auge:

public static Path webRoot;
public static SignMarkers instance;
public static Logger logger;
public static Map<World, MarkerSet> markerSet = new ConcurrentHashMap<>();

Für ein Hobby Projekt irgendwie unschön. Aber da merkt man, da hat jemand wenig von den Keywords verstanden. Ohne jetzt zu bashen, das ist ja nur ein Hobby Projekt. Ich muss mich da auch immer wieder hinterfragen.

public

Macht aus meiner Sicht keinen Sinn alles public zu machen. Normalerweise will man Encapsulation, heisst soviel dass wir unsere Daten in der Klasse nicht öffentlich zugreifbar machen. Mit diesem Prinzip wollen wir erreichen, dass nicht andere Klassen unsere Daten verändern. Dieses Plugin sollte alle Resourcen selber verwalten und braucht definitv seine Daten nicht zu veröffentlichen.

static

Völlig falsche Anwendung von static. Hier wurden alle Felder auf die Klasse gemappt. Also egal welche Instanz ihr da habt, diese Felder werden einen “globalen” Wert haben. Klar läuft dieses Plugin am Schluss nur einmal, doch wenn man wider besseren Wissens alles auf static setzt sind Programmierfehler später unvermeidbar. Ich gehe immer davon aus, dass Felder in der Klasse zur Instanz gehören. Gut heute wird mich natürlich die IDE mit dem Syntaxhighlighting darauf Aufmerksam machen. Trotzdem verwendet nicht `static wenn ihrs nicht versteht.

Logger

Ich bin in meinen Projekten verwöhnt und setzte ausschliesslich auf Slf4j mit logback. Eine solche instanzierung des Loggers habe ich noch nie gesehen. Normalerweise macht man über statische Utility des Loggers eine Instanz:

final Logger logger = LoggerFactory.getLogger(Wombat.class);

Hier darf man auch getrost den static verwenden, da man nicht für jede instanz unbedingt einen Logger braucht. Ich habe für dieses Projekt den Logger einfach von der Plugin Infrastruktur aufgerufen.

Am Schluss habe ich nun alles so umstrukturiert, dass Encapsulation eingehalten wurde. An der Logik selber habe ich nichts geändert. Und siehe da, das Projekt könnte man jetzt Unit-Testen.

In der “Main”-Klasse wird das markerSet instanziert:

private final Map<World, MarkerSet> markerSet = new ConcurrentHashMap<>();

In den Event-Listener Klassen, wird das markerSet als final direkt gebraucht:

private final Map<World, MarkerSet> markerSet;