Sehr einfaches Hangman-Spiel

80372
Alexander

Folgt dieser Code den Standardkonventionen und ist nicht redundant? Wie kann ich es für andere verständlicher machen?

Es ist eine Übung aus dem Java-Einführungsbuch:

Schreiben Sie ein Hangman-Spiel, das zufällig ein Wort generiert und den Benutzer dazu auffordert, jeweils einen Buchstaben zu erraten. Jeder Buchstabe im Wort wird als Stern angezeigt. Wenn der Benutzer eine korrekte Schätzung vornimmt, wird der tatsächliche Buchstabe angezeigt. Wenn der Benutzer ein Wort beendet hat, zeigen Sie die Anzahl der Fehler an.

Mein Programm:

import java.util.Arrays;
import java.util.Scanner;

public class Hangman{
    public static void main(String[] args) {
        String[] words = {"writer", "that", "program"};
        // Pick random index of words array
        int randomWordNumber = (int) (Math.random() * words.length);
        // Create an array to store already entered letters
        char[] enteredLetters = new char[words[randomWordNumber].length()];
        int triesCount = 0;
        boolean wordIsGuessed = false;
        do {
        // infinitely iterate through cycle as long as enterLetter returns true
        // if enterLetter returns false that means user guessed all the letters
        // in the word e. g. no asterisks were printed by printWord
        switch (enterLetter(words[randomWordNumber], enteredLetters)) {
            case 0:
                triesCount++;
                break;
            case 1:
                triesCount++;
                break;
            case 2:
                break;
            case 3:
                wordIsGuessed = true;
                break;
        }
        } while (! wordIsGuessed);
        System.out.println("\nThe word is " + words[randomWordNumber] +
            " You missed " + (triesCount -findEmptyPosition(enteredLetters)) +
            " time(s)");
    }

    /* Hint user to enter a guess letter,
    returns 0 if letter entered is not in the word (counts as try),
    returns 1 if letter were entered 1st time (counts as try),
    returns 2 if already guessed letter was REentered,
    returns 3 if all letters were guessed */
    public static int enterLetter(String word, char[] enteredLetters)    {
        System.out.print("(Guess) Enter a letter in word ");
        // If-clause is true if no asterisks were printed so
        // word is successfully guessed
        if (! printWord(word, enteredLetters))
            return 3;
        System.out.print(" > ");
        Scanner input = new Scanner(System.in);
        int emptyPosition = findEmptyPosition(enteredLetters);
        char userInput = input.nextLine().charAt(0);
        if (inEnteredLetters(userInput, enteredLetters)) {
            System.out.println(userInput + " is already in the word");
            return 2;
        }
        else if (word.contains(String.valueOf(userInput))) {
            enteredLetters[emptyPosition] = userInput;
            return 1;
        }
        else {
            System.out.println(userInput + " is not in the word");
            return 0;
            }
    }

    /* Print word with asterisks for hidden letters, returns true if
    asterisks were printed, otherwise return false */
    public static boolean printWord(String word, char[] enteredLetters) {
        // Iterate through all letters in word
        boolean asteriskPrinted = false;
        for (int i = 0; i < word.length(); i++) {
            char letter = word.charAt(i);
            // Check if letter already have been entered bu user before
            if (inEnteredLetters(letter, enteredLetters))
                System.out.print(letter); // If yes - print it
            else {
                System.out.print('*');
                asteriskPrinted = true;
            }
        }
        return asteriskPrinted;
    }

    /* Check if letter is in enteredLetters array */
    public static boolean inEnteredLetters(char letter, char[] enteredLetters) {
        return new String(enteredLetters).contains(String.valueOf(letter));
    }

    /* Find first empty position in array of entered letters (one with code \u0000) */
    public static int findEmptyPosition(char[] enteredLetters) {
        int i = 0;
        while (enteredLetters[i] != '\u0000') i++;
        return i;
    }
}

Protokoll seiner Arbeit:

(Guess) Enter a letter in word **** > a
(Guess) Enter a letter in word **a* > t
(Guess) Enter a letter in word t*at > q
q is not in the word
(Guess) Enter a letter in word t*at > t
t is already in the word
(Guess) Enter a letter in word t*at > b
b is not in the word
(Guess) Enter a letter in word t*at > h
(Guess) Enter a letter in word that
The word is that You missed 2 time(s)
Antworten
10
Wäre es gut, wenn ich einen Link zum Free-Image-Hosting mit einem Blockbildschema verweise, das die Logik meines Programms veranschaulicht, weil es nicht offensichtlich ist, wie es funktioniert oder nicht? Alexander vor 4 Jahren 0
Flussdiagramme und dergleichen würden die Verständlichkeit dessen, was Sie tun, verbessern. Das ist immer eine gute Sache. Mast vor 4 Jahren 2
Ich denke, Sie haben gute Arbeit geleistet, um Redundanzen zu beseitigen und Methoden zu entwickeln, um sie noch präziser zu gestalten. Es ist jedoch etwas schwer zu folgen. Vielleicht helfen mehr beschreibende Methodennamen (die beste Dokumentation ist lesbarer Code!) dev_feed vor 4 Jahren 2
Ihr Code ist zu kompliziert, Sie müssen erklären, was los ist, da es für einen Schüler schwierig ist zu lesen und genau zu wissen, was passiert. Vielen Dank. vor 3 Jahren 0

2 Antworten auf die Frage

5
nattyddubbs

Magic Numbers: Es ist besser, Ihre Definitionen für 0,1,2,3 in ein enumoder finalFeld zu verschieben, um sie mit einem aussagekräftigen Namen zu referenzieren. Zum Beispiel final int LETTER_NOT_IN_WORD = 0oderenum HangmanGuess { LETTER_NOT_IN_WORD }

Redundanz: Die Kernlogik zwischen findEmptyLettersund inEmptyLettersist die gleiche, die Sie charin einem suchen char[]. Sie könnten so umgestalten:

/* Check if letter is in enteredLetters array */
public static boolean inEnteredLetters(char letter, char[] enteredLetters) {
    return indexOf(letter, enteredLetters) >= 0;
}

/* Find first empty position in array of entered letters (one with code \u0000) */
public static int findEmptyPosition(char[] enteredLetters) {
    return indexOf('\u0000', enteredLetters)
}

/* Determine the index in {@code vals} where {@code ch} exists. Returns -1 if {@code ch} is not in {@code vals}. */
public static int indexOf(char ch, char[] vals) { 
    return Arrays.asList(vals).indexOf(Character.valueOf(ch));
}

Es ist auch darauf hinzuweisen, dass in der aktuellen Implementierung zurückgegeben findEmptyPositionwird, nwo nsich die enteredLetters.length + 1leeren Positionen befinden. Ich bin mir nicht sicher, ob dies die gewünschte Funktionalität ist.

4
dev_feed

Erste (kleine) Sache: Es scheint häufiger zu sein Randomals Math.random(). Siehe diese Best Practices Post.

Die Kommentare und der erste Wechsel innerhalb von do {} while () sind für mich persönlich sehr verwirrend. enterLetter()nie zurück trueoder false, und auch die meisten neuen Programmierer wissen, eine do / while iteriert, bis die Bedingung erfüllt ist. Es wäre hilfreicher zu beschreiben, was 0-3 darstellt (und wenn dies Produktionscode oder komplexer wäre, würde eine Aufzählung es wesentlich verständlicher machen).

Innerhalb von enterLetter()wäre es hilfreich, den int emptyPosition = findEmptyPosition(enteredLetters);Anruf an die Stelle zu verschieben, an der else ifer verwendet wurde. Es verhindert nicht nur die Zuweisung des Speichers bis zur Verwendung, sondern es ist auch lesbarer (und Sie könnten die emptyPositionVariable entfernen, wenn Sie möchten, und die Funktion innerhalb der Referenz aufrufen, aber das ist semantisch).