Bitte um Optimierungsvorschläge / Verbesserungsvorschläge / allgemeines Feedback

Vivien

Mitglied
Hallo Liebe Community,

ich habe ein kleines Programm geschrieben & würde Euch gerne um Eure Rückmeldungen bitten unter anderem zu:
  • Welche Stellen könnten optimiert werden & wie hätten Sie besser gelöst werden können?
  • Was ist unübersichtlich geworden?
  • Was hätte ich besser machen können? (Stil, unnötiger Code, Fehler, alles andere was euch noch an Kritik einfällt etc etc)
  • Was ist gut gelungen & sollte beibehalten werden & was auf jeden Fall schnell vergessen werden?
Hintergrund ist, dass ich alleine Java lerne & nicht möchte, dass sich unschöne oder gar falsche Angewohnheiten bei mir einprägen, gerade zu Beginn. 🤔
Meine Aufgabenstellung lautete:

Ratespiel:
Das Spiel umfasst ein Ratespiel-Objekt und eine beliebige Anzahl an Spielern. Das Spiel generiert eine Zufallszahl zwischen 0 und 9, und die Spieler versuchen, diese Zahl zu erraten. (Klar, ich weiß - ist nicht sonderlich spannend😅)
Klassen:
Ratespiel.class
Spieler.class
SpielStarter.class
Logik:
1)Die Klasse SpielStarter ist die Klasse, mit der die Anwendung beginnt. Sie hat die main()- Methode.
2) ln der main()-Methode wird ein Ratespiel-Objekt erzeugt und seine starteSpiel()-Methode aufgerufen.
3) in der starteSpiel()-Methode des Ratespiel-Objekts wird das vollständige Spiel abgewickelt. Sie »denkt<< sich eine Zufallszahl aus (das von den Spielern zu erratende Ziel). Dann fordert sie jeden Spieler auf zu raten, prüft die Ergebnisse und gibt dann entweder Informationen zu dem/den erfolgreichen Spieler(n) aus oder fordert sie auf, erneut zu raten.

Meine Lösung:

[CODE lang="java" title="Ratespiel.java"]import java.util.*;

public class Ratespiel {
public void starteSpiel() {
//Spieler anlegen
Spieler spieler = new Spieler();
spieler.SpielerAnlegen();

//Formatierung
System.out.println();
System.out.println("Spiel startet jetzt!");
System.out.println();

//Zufallszahl Intervall festlegen
int max = 9;
int min = 0;
int range = max - min + 1;

boolean RichtigeAntwortVorhanden = false;
do {
//Zufallszahl erzeugen im oben festgelegten Intervall
int rand = 0;
for (int i = 0; i < 10; i++) {
rand = (int)(Math.random()*range) + min;
}
//alt get
//System.out.println("Alle " + Spieler.getSpielerAnzahl() + " Spieler suchen sich jetzt eine Zahl aus:");
System.out.println("Alle " + Spieler.getSpielerAnzahlAlternativ() + " Spieler suchen sich jetzt eine Zahl aus:");

//Speichere SpielerAnzahl in Variable um in for Schleife darauf zugreifen zu können
int SpielerAnzahl = Spieler.getSpielerAnzahlAlternativ();
//LEge ArrayListe an, um eingegebene Spielerzahlen abzuspeichern
ArrayList<Integer> SpielerZahlenListe = new ArrayList<Integer>();


//Frage nach den Zahlen und speichere in der ArrayList
for (int i = 1; i <= SpielerAnzahl; i++) {
System.out.println("Spieler " + i + " such dir eine Zahl zwischen 0 und 9 aus!");
Scanner gewaehlteZahl = new Scanner(System.in);
SpielerZahlenListe.add(gewaehlteZahl.nextInt());
}
System.out.println("____________________________________");
//Prüfen welche Spieler richtig lagen


for (int i = 0; i < SpielerAnzahl; i++) {
int Spielernummer = i+1;
if (SpielerZahlenListe.get(i) == rand) {
System.out.println("Spieler " + Spielernummer + " lag richtig & hat gewonnen!");
RichtigeAntwortVorhanden = true;
}

else {
System.out.println("Spieler " + Spielernummer + " hat leider nicht richtig geraten...");
}
}

System.out.println("Die richtige Zahl lautete: " + rand);
if (RichtigeAntwortVorhanden == false) {
System.out.println("__________________DURCHGANG BEENDET______________");
System.out.println("Kein Spieler hat richtig geraten! Ihr dürft noch einmal raten!");
System.out.println("Nächster Durchgang beginnt:");
}
} while (RichtigeAntwortVorhanden == false);
}
}[/CODE]

[CODE lang="java" title="Spieler.java"]import java.util.*;

public class Spieler {
//Alternativ get
//public static int SpielerAnzahl;
public static int anzahlSpielerAusUserInput;

public void SpielerAnlegen() {
//Einlesen der Spielerzahl
Scanner anzahlSpieler = new Scanner(System.in);
System.out.println("Wie viele Spieler seid Ihr? Bitte gebt die Zahl der Mitspieler ein!");
anzahlSpielerAusUserInput = anzahlSpieler.nextInt(); //Schreibe userinput in variable anzahlSpielerAusUserInput
if (anzahlSpielerAusUserInput > 0) {
System.out.println("Spiel mit " + anzahlSpielerAusUserInput + " Spielern(n) wird erstellt!");

//Erstelle ArrayListe mit eingegebenen Spielern und gib jedem Spieler eine Nummer angefangen bei 1 & endend bei Spielernr. aus UserInput
ArrayList<Integer> SpielerArray = new ArrayList<Integer>();
for (int i=1; i<=anzahlSpielerAusUserInput; i++) {
SpielerArray.add(i);
//alt. get
//SpielerAnzahl++;
}

//Ausgabe der erstellten Spieler aus der ArrayListe mit dem ListIterator (mehr zur Überprüfung, das auch alle Spieler korrekt erstellt wurden sind)
ListIterator<Integer> li = SpielerArray.listIterator();

while(li.hasNext()) {
System.out.println("Spieler Nummer " + li.next() + " erfolgreich erstellt.");
}
}
//Wenn Spieleranzahl nicht 0 ist, PRogramm abbrechen
else {
System.out.println("Spieleranzahl muss größer 0 sein!");
System.exit(0);
}
}

//alt get
/*public static int getSpielerAnzahl() {
return SpielerAnzahl;
}*/
public static int getSpielerAnzahlAlternativ() {
return anzahlSpielerAusUserInput;
}



}[/CODE]

[CODE lang="java" title="SpielStarter.java"]public class SpielStarter {
public static void main(String[] main) {
Ratespiel ratespiel = new Ratespiel();
ratespiel.starteSpiel();
}
}[/CODE]

Ich weiß es ist schon ein wenig mehr Code und klar das Programm läuft, wäre euch aber dennoch wie gesagt sehr dankbar für das oben genannte Feedback.
Aus anderen Augen oder welchen mit mehr Erfahrung dürfte der Code bestimmt auch wieder anders aussehen 😇

Ich freu mich auf eure Kritik & Lg
 
1. alles was nicht static final oder main enthält sollte eig nicht static sein
2. java unterstützt dieses tag /** /* dh wenn du ovn irgendwo die Funktion anschaust wird dir dieses Kommentar angezeigt in der IDE, dieses Kommentar sollte auch das einzige sein in der Methode denn wenn du merkst dass dieses Kommentar 5 Sachen umfasst dann weist du dass du eine Gott Methode gebaut hast also die gefühlt alles kann was nicht dem java grundsatz entspricht
3. deine Einrückungen machen den Code Schwer lesbar, immer wenn du eine { aufmachst solltest du 4 Leerzeichen was zb in Eclipse ein Tab ist einrücken und auf dem Level shcließt sich die Klammer dann auch...es gibt eine Einstellung wo tab 4 leerzeichen macht anstatt eines tab zeichens was auch sinn macht da das tab zeichen untershciedlich lang ist aber der code sollte ja überall gleich ordentlich ausschauen

Für mich waren die Kommentare bei 2. immer wichtig für andere sinds bestimmt unwichtig nur ich hasse externe Dokumentationen einfach... und java hat da halt ne schicke Option

EDIT:

das wäre ein Beispiel für punkt 2. das siehst du überall im code wenn du bei deiner IDE durch die Verfügbaren Methoden durchgehst falls du halt bei der Methode bist die dieses Kommentar hat


mal 3 Beispiele...ob das jetzt dne Normen entsprechend richtig ist ist egal....es zeigt dir halt ob du deine Methoden zu voll gepackt hast oder nicht

wenn man das was in der Methode nicht erklären kann ist es zu komplex / zu viel / zu unübersichtlich ...dh aber nicht dass du das überall machen sollst zb getter und setter sind allgemein bekannt was sie machen sollten

Java:
      /**
       * Increases the amount counter of a specific card<br>
       * If that card doesn't exist yet it will be created with counter=1<br>
       * @apiNote Max Card Counter == the Limitation of that specific card
       * @param name of a card
       */

Java:
        /**
         * @apiNote This Node is reserved for the Hover handler of the leftInfoBox<br>
         * @return Gets the card image as ImageView
         */
Java:
        /**
         * Possible Values:<br>
         * "main"<br>
         * "additional"<br>
         * "commander"<br>
         * @apiNote Limitation for these Values not implemented
         * @return the deck type in which it should be
         */
 
Zuletzt bearbeitet von einem Moderator:
Ich finde kurze Methoden mit prägnanten Namen, die einfach zu lesen und verstehen sind, gut. (*)

Das kann man prinzipiell auch etwas üben indem man sich klare Grenzen vorgibt a.la. nicht mehr wie x Zeilen Code pro Methode.

(*) Das, was ich gut finde, findet sich in abgewandelter Form alles in diversen Clean Code Ableitungen und lässt sich ganz umfangreich begründen. Ich habe es aber bewusst so einfach ausgedrückt um es einfach als Anregung mit zu geben.
 
public class Spieler { //Alternativ get //public static int SpielerAnzahl; public static int anzahlSpielerAusUserInput; public void SpielerAnlegen() {
Deine Klasse Spieler ist keine Klasse im Sinne der OOP, Kaspelung, Polymorphie usw.

Darin taucht wieder IO mit Scanner auf.

Beim professionellen Design trennt man fachliche Logik von OOP.

Dies ist für Test (TDD) notwendig, sollte aber auch zu einem besseren Design führen.

Variable sollten klein geschrieben werden.

Ordentliche Einrückung hilft auch (in Eclipse mit Strg-A und Strg-I).
 
Vielen Dank @Mart !
1. ) Habe das mit den Dokumentationskommentaren noch gar nicht gewusst 😳 Werde sie aber in Zukunft nutzen. Dankeschön auch für die Beispiele!
2. ) Mit dem static, trotz mehrmaligem lesen haut das aus irgendeinem Grund bei mir immer noch nicht so gut hin. Wenn ich static aus Zeile 6 & 42 in Spieler.java entferne, erhalte ich beim kompilieren von Ratespiel.java folgendes:

[CODE lang="java" title="Ratespiel.java (error beim kompilieren)"]Ratespiel.java:28: error: non-static method getSpielerAnzahlAlternativ() cannot be referenced from a static context
System.out.println("Alle " + Spieler.getSpielerAnzahlAlternativ() + " Spieler suchen sich jetzt eine Zahl aus:");
Ratespiel.java:31: error: non-static method getSpielerAnzahlAlternativ() cannot be referenced from a static context
int SpielerAnzahl = Spieler.getSpielerAnzahlAlternativ();[/CODE]

Ich vermute das hat etwas damit zu tun, dass ich in den Zeilen 28 & 31 mittels Spieler.getSpielerAnzahlAlternativ() auf int anzahlSpielerAusUserInput zugreife? Leider weiß ich nicht wie ich dieses Problem beheben könnte ohne static zu verwenden 😣

3.) @Mart & @Barista
Mit dem Tab, werde ich Heute gleich mal schauen wie ich gedit entsprechend einstellen kann, dass der Tab 4 Leerzeichen ergibt 🙂 Dann wäre das auch für mich selbst übersichtlicher 😃 Dankeschön.
EDIT:
Für alle die es auch interessiert der Link zu: gedit auf 4 Leereichen umstellen

Vielen Dank auch @kneitzel !
Ich werde nochmal als Übung das ganze Programm durchgehen & es in kürzeren Methoden umsetzen (oder ggf. versuchen noch einmal besser zu schreiben). Wären so 8-10 Zeilen je Methode bei diesem Programm angemessen? 🤔

@Barista Vielen Dank für den Variablenhinweis - das hat sich doch tatsächlich irgendwie ungewollt eingeschlichen ... 😑
Könntest du vielleicht die beiden Sätze:

Beim professionellen Design trennt man fachliche Logik von OOP
&
Deine Klasse Spieler ist keine Klasse im Sinne der OOP, Kaspelung, Polymorphie usw
ein wenig näher erklären oder wenn es den Rahmen sprengen würde 1-2 gute Links mit auf den Weg geben? 🤨😊 Vielen Dank 🙂
 
Zuletzt bearbeitet:
Ich schrieb:

Beim professionellen Design trennt man fachliche Logik von OOP

Hier war ich geistig mit dem Tippen schon fertig.

Ich meinte eigentlich:

Beim professionellen Design trennt man fachliche Logik von IO (Input/Output).

Stell Dir vor, die fachliche Logik wäre einigermassen umfangreich.

Du möchtest zum Beispiel absichern, dass bei richtig geratener Zahl dies auch als richtig erkannt wird.

Dazu muss man leider die zufällige Quelle rausnehmen, das wäre üblicherweise Depency Injection.

In diesem Fall einer Factory-Methode zum Erzeugen von Spiel-Instanzen eine Instanz geben, die ein Interface mit einer getRandomZahl-Methode implementiert.

Für den Test gibt man eine feste Zahl zurück, im echten Code (produktiv) eine Zufallszahl.

Bei Dir ist der ganze Status des Spiels nur eine Zahl, stell Dir vor, es wäre ein Spielfeld.

Dann gibt es eine Methode des Spiel(feld)es, welches die Benutzereingabe entgegen nimmt und mit Erforlg/Nicht-Erfolg antwortet.

Dies könnte jetzt der Test absichern, also dass die Funktionlität auch nach Änderungen im Code noch vorhanden ist.

Im echten Code (produktiv) wird die Eingabe-Methode mit der Benutzeroberfläche verbunden.

In Deinem Code ist auch noch der Ablauf mit der Logik verwoben.

Hättest Du eine GUI (Swing, FX, Web oder so) würde das nicht klappen, Du kannst den Fluss des Programmes nicht einfach anhalten.

Also musst Du dann den Ablauf auf der GUI (grafisches User Interface) explizit managen (Oder Du wartest auf Loom, ich nehme an, da gibt es so was wie Co-Routinen)

Zu OOP gibt es im Web jede Menge Infos, bitte mal suchen.
 
Ich schrieb:

Deine Klasse Spieler ist keine Klasse im Sinne der OOP, Kaspelung, Polymorphie usw

Stell Dir Klassen wie Personen oder Geräte vor, die haben einen internen Zustand und Schnittstellen nach aussen.
 
Vielen Dank @Mart !
1. ) Habe das mit den Dokumentationskommentaren noch gar nicht gewusst 😳 Werde sie aber in Zukunft nutzen. Dankeschön auch für die Beispiele!
2. ) Mit dem static, trotz mehrmaligem lesen haut das aus irgendeinem Grund bei mir immer noch nicht so gut hin. Wenn ich static aus Zeile 6 & 42 in Spieler.java entferne, erhalte ich beim kompilieren von Ratespiel.java folgendes:

[CODE lang="java" title="Ratespiel.java (error beim kompilieren)"]Ratespiel.java:28: error: non-static method getSpielerAnzahlAlternativ() cannot be referenced from a static context
System.out.println("Alle " + Spieler.getSpielerAnzahlAlternativ() + " Spieler suchen sich jetzt eine Zahl aus:");
Ratespiel.java:31: error: non-static method getSpielerAnzahlAlternativ() cannot be referenced from a static context
int SpielerAnzahl = Spieler.getSpielerAnzahlAlternativ();[/CODE]

Ich vermute das hat etwas damit zu tun, dass ich in den Zeilen 28 & 31 mittels Spieler.getSpielerAnzahlAlternativ() auf int anzahlSpielerAusUserInput zugreife? Leider weiß ich nicht wie ich dieses Problem beheben könnte ohne static zu verwenden 😣

3.) @Mart & @Barista
Mit dem Tab, werde ich Heute gleich mal schauen wie ich gedit entsprechend einstellen kann, dass der Tab 4 Leerzeichen ergibt 🙂 Dann wäre das auch für mich selbst übersichtlicher 😃 Dankeschön.
EDIT:
Für alle die es auch interessiert der Link zu: gedit auf 4 Leereichen umstellen

Vielen Dank auch @kneitzel !
Ich werde nochmal als Übung das ganze Programm durchgehen & es in kürzeren Methoden umsetzen (oder ggf. versuchen noch einmal besser zu schreiben). Wären so 8-10 Zeilen je Methode bei diesem Programm angemessen? 🤔

@Barista Vielen Dank für den Variablenhinweis - das hat sich doch tatsächlich irgendwie ungewollt eingeschlichen ... 😑
Könntest du vielleicht die beiden Sätze:


&

ein wenig näher erklären oder wenn es den Rahmen sprengen würde 1-2 gute Links mit auf den Weg geben? 🤨😊 Vielen Dank 🙂
da du gedit benutzt sind die Kommentare so für die katz wie ich sie geschrieben habe eclipse usw unterstüzen das kommentieren in dieser form... ich kann aber nicht gedit empfehlen gnome hat da auf ganzer linie versagt einen guten editor zu bauen sobald code ins spiel kommt fängt gedit zum lagen an und stürzt ab durch die Farb hervor hebungen es wird später einfach nur zum kotzen nur so als hinweis...wenn man schon alles selber machen möchte würde ich emacs und vim empfehlen aber ich seh keinen sinn sich selber steine in den weg zu legen
 

Zurück
Oben