Entspricht dieser Code Standardkonventionen, nicht redundant? Wie kann ich es für andere verständlicher machen?

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

Schreiben Sie a Hangman-Spiel, das zufällig ein Wort generiert und den Benutzer auffordert, jeweils einen Buchstaben zu erraten. Jeder Buchstabe im Wort wird als Sternchen angezeigt. Wenn der Benutzer eine korrekte Vermutung vornimmt, wird der tatsächliche Buchstabe angezeigt. Wenn der Benutzer a beendet hat Wort, 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)  

Kommentare

  • Wäre es gut, wenn ich einen Link zum Free-Image-Hosting mit einem Bild des Blockschemas anhängen würde Logik meines Programms, weil ' nicht offensichtlich ist, wie es funktioniert, oder ' nicht erforderlich ist?
  • Flussdiagramme und dergleichen würden th erhöhen Die Verständlichkeit dessen, was Sie ' tun. Das ' ist immer eine gute Sache.
  • Ich denke, Sie ' haben gute Arbeit geleistet, um Redundanzen zu beseitigen und zu erstellen Methoden, um es prägnanter zu machen. Es ist jedoch etwas schwierig zu folgen. Vielleicht helfen aussagekräftigere Methodennamen (die beste Dokumentation ist lesbarer Code!)
  • Ihr Code ist zu kompliziert. Sie müssen erklären, was vor sich geht, da es für einen Schüler schwierig ist, genau zu lesen und zu wissen, was passiert ' passiert. Vielen Dank.

Antwort

Magische Zahlen: Es wäre besser, Ihre Definitionen für 0,1 zu verschieben. 2,3 in ein enum oder final -Feld, damit Sie sie mit einem aussagekräftigen Namen referenzieren können. Zum Beispiel final int LETTER_NOT_IN_WORD = 0 oder enum HangmanGuess { LETTER_NOT_IN_WORD }

Redundanz: Die Kernlogik zwischen findEmptyLetters und inEmptyLetters ist dasselbe. Sie suchen nach einer char in einer char[] . Sie könnten Folgendes 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)); } 

Beachten Sie auch, dass in der aktuellen Implementierung findEmptyPosition gibt n zurück, wobei n die enteredLetters.length + 1 ist, wenn keine leeren Positionen vorhanden sind. Ich bin mir nicht sicher, ob dies die gewünschte Funktionalität ist.

Antwort

Erste (kleine) Sache: Es scheint üblicher zu sein Verwenden Sie Random als Math.random(). Siehe diesen Best Practices Beitrag.

Die Kommentare und der Wechsel als erstes in do {} while () sind für mich persönlich sehr verwirrend. enterLetter() gibt niemals true oder false, und selbst die meisten neuen Programmierer kennen eine do / while-Iteration, bis die Bedingung erfüllt ist. Es wäre hilfreicher zu beschreiben, was 0-3 darstellt (und wenn dies der Fall ist) Wäre der Produktionscode oder komplexer, würde eine Aufzählung ihn wesentlich verständlicher machen.

Innerhalb von enterLetter() wäre es hilfreich, die int emptyPosition = findEmptyPosition(enteredLetters); ruft die else if auf, in der sie verwendet wird. Dies verhindert nicht nur die Zuweisung des Speichers, bis er verwendet wird, sondern ist auch besser lesbar (und Sie können die Variable emptyPosition entfernen, wenn Sie möchten, und die Funktion in der Referenz aufrufen , aber das ist semantisch).

Schreibe einen Kommentar

Deine E-Mail-Adresse wird nicht veröffentlicht. Erforderliche Felder sind mit * markiert.