Ist das Programm schlecht bzw. schlampig programmiert ?

[Käsekuchen]

import java.util.Scanner;

public class Galgenmännchen {

    
    
    
    public static String wörter() {
        String [] wörter = {"Mercedes","BMW","Audi","Müller","Programmieren","Skateboard"};
        int random = (int) (Math.random()*5)+1;
        String wort = wörter[random];
               wort = wort.toUpperCase();
        System.out.println("Ihr Wort hat "+wort.length()+" Buchstaben");
        return wort;
        
    }
    public static void XXX(String wort) {
        char [] wörter1 = wort.toCharArray();
        char [] striche = new char [wörter1.length];
        
        
        for(int i = 0;i<striche.length;i++) {
            striche[i]='-';
            System.out.print(striche[i]);
        }
        int gewonnen =0;
        for(int a = 1; a<16;a++) {
            System.out.println();
            boolean buchstabeGefunden = false;
        
        Scanner s = new Scanner (System.in);
        System.out.println("geben Sie ein Buchstabe ein");
        String buchstabe = s.nextLine();
               buchstabe = buchstabe.toUpperCase();
        char x = buchstabe.charAt(0);
        
        for(int h = 0;h<striche.length;h++) {
            
            if(wörter1[h]==x) {
                striche[h]=x;
                buchstabeGefunden=true;
                gewonnen++;
                
            }
            
        }
        System.out.println("Das war ihr "+a+" Versuch von insgesamt 15");
        for(int k =0; k<striche.length;k++) {
        System.out.print(striche[k]);
        }
        
        System.out.println();
        if(buchstabeGefunden)
            System.out.println("Sehr gut, Sie haben haben einen Buchstaben gefunden");
        else System.out.println("Sie haben haben keinen Buchstaben gefunden");

        
        if(gewonnen==striche.length) {
            System.out.println("Sie haben gewonnen");
            break;
        }
        if(a==15) {
            System.out.println("Sie haben die maximale Anzahl an Versuchen erreicht, Sie haben verloren");
            break;
        }
        
    }
    
    }
    public static void main (String[]args) {
        String wortV =wörter();
        XXX(wortV);
        
        
    }
}

 

[LimDul]

Ich hab gerade mal Zeit. Dinge die mir auffallen:

• Umlaute in Klassen, Methoden oder Variablen sollte man vermeiden
• Namen sollten verständlich sein.
  • Was macht eine Methode XXX? Methodennamen sollten mit Kleinbuchstaben anfangen.
  • Warum heißt das Array wörter1? Gibt es ein wörter2? Und warum heißt das Array was mit wörtern, wenn es Zeichen und nicht wörter enthält? Wäre nicht buchstaben sinnvoller?
  • Die Schleifenvariablen sehen auch gewürfelt aus - a, h und k. Warum? Warum nicht überall, wie man es normalerweise macht i?
  • Warum heißt die Variable in main wortV? Was bedeutet V? Warum nicht einfach gesuchtesWort
  • Warum heißt die Methode wörter, aber sie liefert ein Wort und keine wörter zurück? Vorschlag getSuchendesWort
• Die Einrückung beim if statement zu buchstabe gefunden sieht gewürfelt hast - das statement hinter dem else gehört in eine eigene Zeile, analog eingerückt zu dem statement hinter dem if
• Man sollte Magic Numbers vermeiden. Es gibt eine Schleife die bis < 16 läuft. Was die 16 bedeutet wird dann bei der Aussage mit den 15 Versuchen klar. Sinnvoller wäre es eine Konstante int MAX_VERSUCHE = 15 zu definieren und die sowohl bei der Schleife, als auch in der Ausgabe zu nutzen. So muss man, wenn man die Zahl der Versuche ändern will, zwei Stellen anpassen
• Den Scanner sollte man außerhalb der Schleife erzeugen und nicht jedes mal neu.

Ansonsten sieht das nach einen klassischen Anfänger Programm aus - und dafür sieht es nicht schlecht aus, viele meiner Anmerkungen sind Formalismen, die man mit der Zeit sich aneignet. Das Programm ist strukturiert, hat einen klaren, nicht zu komplexen Ablauf.

 

[KonradN]

Es fallen mehrere Dinge auf:

a) es ist nicht objektorientiert. Hier ist die Frage, in wie weit das etwas ist, das Du schon gehabt hast / können solltest. Aber Java ist eine objektorientierte Sprache und da sollte man dann auch objektorientiert entwickeln.

b) Du hast alles gerade mal in zwei Methoden. Da ist vor allem die Zweite viel zu groß und unübersichtlich.

c) Bezeichner - hier solltest Du deutlich mehr Wert auf gute Bezeichner legen. Dazu zählen Punkte wie:
c1) Keine Umlaute in den Namen
c2) Methoden sollten sagen, was sie machen. Dazu ist dann in der Regel ein Verb mit im Namen. Die erste Methode gibt z.B. ein zufälliges Wort zurück. Das wäre also etwas wie getRandomWord oder so. (Da Java selbst in Englisch ist, wird sehr gerne auf englische Bezeichner zurück gegriffen. Das muss aber natürlich nicht sein. Etwas wie holeZufaelligesWort wäre also auch ok.
c3) Namen wie i, a, s, .... sagen nichts aus. Da wirklich darauf achten, dass der Name aussagekräftig ist. Und Namen sollten nicht irreführend sein: wörter1 enthält ja keine Wörter sondern die Zeichen eines Wortes....

e) Logik - hier sollte man aufpassen, dass man möglichst keine festen Werte im Code hat. Man spricht da gerne von magic Numbers. Beispiel hier:
int random = (int) (Math.random()*5)+1;
Das ist ganz nebenbei falsch - denn Du bekommst einen Zufallswert von 1..5 - aber du willst ja ein zufälliges Wort aus dem Wörter-Array haben. Das hat 6 Elemente mit einem Index von 0..5. Aber das Problem ist auch, dass Du schnell eine Änderung an einer Stelle machst und die zweite Stelle vergisst. Also Du nimmst einen Wert aus dem Array heraus und schon bekommst Du eine Exception, wenn ein nicht vorhandener Index durch Zufall gewählt wird.
Somit wäre da etwas wie int random = (int) (Math.random() * wörter.length); denkbar.

f) Design - da kann man natürlich sehr viel machen. Da kommen mir so Dinge in den Sinn, wie Verantwortlichkeiten aufteilen. Eine Methode, die ein zufälliges Wort zurück gibt muss dieses ja nicht ausgeben. Das würde eine Spiellogik machen. Denn das ist ja prinzipiell unabhängig: Das eigentliche Spiel funktioniert mit beliebigen Frontend. So könntest Du z.B. entscheiden, dass Du es als GUI Applikation machst oder so.

 

[Robert Zenz]

Jein. Formatierung ist furchtbar (einmal von der IDE formatieren lassen), Namen sind teilweise furchtbar ("a", "s", "x"?), Praesentation und Logik sind vermischt. Du instanszierst eine Instanz, verwendest dann aber nur statische Methoden. Fuer einen Anfaenger in Ordnung, wuerde ich sagen.

Was die Namen angeht, bennene Dinge immer nachdem was sie halten oder tun. Eine Grundregel von mir ist "Man darf einzelne Buchstaben nur dann als Namen verwenden, wenn man es mit Dimensionen (x, y, z, ...) zu tun hat."

Was du willst ist die Logik von der Praesentation, in dem Fall der Ausgabe auf der Kommandozeile, etwas zu trennen. Also fangen wir mal damit an dass die Funktion wörter eigentlich "waehleZufaelligesWort" sein sollte:

private final String[] WOERTER = new String[] { /* ... */ };

private static String waehleZufaelligesWort() {
    int wortIndex = new Random().nextInt(WOERTER.length);
    
    return WOERTER[wortIndex];
}

Die Ausgabe welches Wort es nun ist, willst du in der Hauptfunktion haben:

public static void spiele() {
    String gesuchtesWort = waehleZufaelligesWort();
    
    System.out.println("Das gesuchte Wort hat " + gesuchtesWort.length() + " Buchstaben.");

    // ...
}

Der Einfachheit halber wuerde ich das geratene Wort einfach als String verwenden. Ist jetzt unmittelbar nicht ganz so huebsch, loest aber ein paar Probleme die man sonst haette (insbesondere mit Fremdsprachen, da ein char unter Umstaenden nicht alle Zeichen abbilden kann). Auszerdem tut man sich dann mit der Ausgabe leichter.

public static void spiele() {
    String gesuchtesWort = waehleZufaelligesWort();
    
    System.out.println("Das gesuchte Wort hat " + gesuchtesWort.length() + " Buchstaben.");

    String geratenesWort = "-".repeat(gesuchtesWort.length()); // "repeat" ab Java 11.
}

Ein testen ob der Buchstabe vorkommt ist sehr simpel:

String geratenerBuchstabe = scanner.nextLine().substring(0, 1).toUpperCaser();

if (gesuchtesWort.contains(geratenerBuchstabe)) {
    // Ja
} else {
    // Nein
}

Das auffuellen im geratenen Wort ist dann etwas komplexer, weil man von Stelle zu Stelle springen muss:

int buchstabenIndex = -1;

while((buchstabenIndex = gesuchtesWort.indexOf(geratenerBuchstabe, buchstabenIndex) != -1) {
    geratenesWort = geratenesWort.substring(0, buchstabenIndex) + geratenerBuchstabe + geratenesWort.substring(buchstabenIndex + 1);
}

Und so weiter...

 

[White_Fox]

Ist das Programm schlecht bzw. schlampig programmiert ?​

Kurz: ja.

Alleine schon deutschen Quelltext finde ich furchtbar.*
Ich selber versuche Quelltext immer so zu schreiben, daß man den Code als Fließtext lesen kann, so daß aus jeder Zeile eindeutig hervorgeht was gerade passieren soll. Methoden benutze ich dann nicht mehr nur, um wiederkehrenden Code abzukürzen, sondern auch um mehrere Schritte zusammenzufassen. Oft mache ich das so, daß ich zuerst oben die Grundzüge grob formuliere und mich dann immer weiter vorarbeite.

Kleines Beispiel: Wie du siehst ist nur eine Methode public, alles andere ist private. Die Methode play() könntest du ins Unendliche aufblasen, stattdessen ist sie ein prägnanter Dreizeiler wobei klar ist, was in jeder Zeile so im Groben passiert. Wenn man genau wissen will was in prepareMatchfield() passiert, schaut man sich die eben an. Das hat auch den Vorteil daß du einen Fehler sehr schnell eingrenzen kannst. Wenn du maximal drei Spieler zulassen willst und es können sich trotzdem vier Spieler anmelden, dann ist eigentlich schon sofort klar, wo der Fehler wahrscheinlich nicht liegen sollte. Dito wenn du etwas nachträglich ändern willst, z.B. die Spieleranzahl ändern. Du findest dich da sehr viel schneller und besser zurecht als in einer monolithischen Riesenmethode.

public class Game{
    private MatchField matchfield;
    
    public void play(){
        prepareMatchfield();
        runGame();
        celebrateWinner();
    }
    
    private prepareMatchfield(){
        waitForPlayers();
        distributeCards();
    }
    
    private runGame(){
        boolean gameOver = false;
        while(!gameOver){
            //...
        }
    }
    
    private celebrateWinner(){
        playWinAnimation();
        humilateLoosers();
    }
}

*Auf Deutsch habe ich auch mal programmiert, da ich eigentlich ein großer Freund der eigenen Muttersprache bin und es z.B. immens lächerlich finde, wenn Deutsche untereinander Englisch sprechen. Im Quelltext auf englische Wörter zu verzichten ist aber unmöglich, mindestens sowas wie if, switch, while, ... kann man ins Deutsche übersetzen, zumindest nicht ohne großen Aufwand zu betreiben und selbst dann muß ich sagen, daß im Englischen deutlich mehr Sinn in weniger Zeichen untergebracht werden kann. Für manche Dinge gibt es gar keine direkte deutsche Übersetzung, versuche mal 'switch' ins Deutsche zu holen.
Es hat aber meines Erachtens doch ein paar erhebliche Nachteile: Manchmal will man doch in internationalen Foren nachfragen, oder Entwickler externer Bibliotheken, da kann mit deinem Code niemand etwas anfangen. Und Deutsch-Englich zu programmieren finde ich mittlerweile schlimm, dann lieber Deutsch und Englisch sauber getrennt.

 

[mihe7]

Hier ist eigentlich schon alles gesagt, aber ich schätze mal, dass das ein Test ist. Im Code ist Whitespace wie Kraut und Rüben im Code verteilt, mal werden Klammern gesetzt, mal keine, mal werden Bezeichner groß geschrieben, mal klein, mal mit Umlaut... Hier scheint mir mit Absicht gegen jede Code-Convention verstoßen worden zu sein, die es jemals gab 🙂

 

[Oneixee5]

Hier ist eigentlich schon alles gesagt, aber ich schätze mal, dass das ein Test ist. Im Code ist Whitespace wie Kraut und Rüben im Code verteilt, mal werden Klammern gesetzt, mal keine, mal werden Bezeichner groß geschrieben, mal klein, mal mit Umlaut... Hier scheint mir mit Absicht gegen jede Code-Convention verstoßen worden zu sein, die es jemals gab 🙂

Ich denke auch so, der Post sollte einfach nur provozieren.

 

[Robert Zenz]

@mihe7 @Oneixee5 Hey, Leute, kommt mal biszchen runter. Man muss hier keine Verschwoerungstheorien stricken (immerhin hat OP nicht auf Konrad reagiert 😉 ).

 

[White_Fox]

Bei allem Respekt worauf @mihe7 so achtet, ich sehe da auch keine Provokation. Wenn ich an meine ersten Programmierversuche denke, dann sah das nicht viel anders aus.

 

[mihe7]

Leute, ich bin absolut tiefenentspannt, habe niemandem Provokation oder sonstwas unterstellt, sondern lediglich angemerkt, dass mir der Code in Verbindung mit der Fragestellung nach einer gestellten Aufgabe aussieht. Der Eindruck kann natürlich täuschen.

Wenn ich an meine ersten Programmierversuche denke, dann sah das nicht viel anders aus.

Vermutlich war er sogar schlechter?

Aber ja, das ist der Punkt, um den es geht: auf der einen Seite sieht man schöne Dinge, wie sprechende Bezeichner "wörter", "striche", "gewonnen" oder gar "buchstabeGefunden". Ein if, das ein Boolean ohne == true abprüft. Und auf der anderen Seite sieht man z. B. völlig zufällig verteilten Whitespace, die Missachtung von Einrückungen, eine Methode XXX oder eine Variable wortV, sowie jede erdenkliche Variante von ifs: mit Block, ohne Block, in einer Zeile, in mehreren Zeilen, das else... usw. Die Mischung machts, warum ich hier an eine Aufgabe denke.