Questo codice segue le convenzioni standard, non è ridondante? Come posso renderlo più comprensibile per altre persone?

È un esercizio tratto dal libro introduttivo di Java:

Scrivi un gioco dellimpiccato che genera una parola in modo casuale e richiede allutente di indovinare una lettera alla volta. Ogni lettera nella parola viene visualizzata come un asterisco. Quando lutente fa unipotesi corretta, viene visualizzata la lettera effettiva. Quando lutente termina un word, mostra il numero di errori.

Il mio programma:

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; } } 

Log del suo lavoro:

 (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)  

Commenti

  • Sarebbe opportuno allegare un collegamento allhosting di immagini gratuite con unimmagine dello schema a blocchi che illustra logica del mio programma, perché ' non è ovvio come funziona o ' non è necessario?
  • Diagrammi di flusso e simili aumenterebbero th la comprensibilità di ciò che ' stai facendo. ' è sempre una buona cosa.
  • Penso che ' abbia fatto un buon lavoro rimuovendo le ridondanze e creando metodi per renderlo più conciso. Tuttavia è un po difficile da seguire. Forse nomi di metodo più descrittivi potrebbero aiutare (la migliore documentazione è il codice leggibile!)
  • il tuo codice è troppo complicato per cui devi spiegare cosa sta succedendo, poiché è difficile per uno studente leggere e sapere esattamente cosa ' sta accadendo. Grazie.

Risposta

Numeri magici: sarebbe meglio spostare le definizioni per 0,1, 2,3 in un campo enum o final in modo da poterli fare riferimento con un nome significativo. Ad esempio final int LETTER_NOT_IN_WORD = 0 o enum HangmanGuess { LETTER_NOT_IN_WORD }

Ridondanza: la logica principale tra findEmptyLetters e inEmptyLetters è lo stesso, stai cercando un char in un char[] . Potresti eseguire il refactoring in questo modo:

/* 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)); } 

Una cosa da sottolineare è anche che nellattuale implementazione, findEmptyPosition restituirà n dove n è enteredLetters.length + 1 se non ci sono posizioni vuote. Non sono sicuro che questa sia la funzionalità desiderata.

Risposta

Prima (piccola) cosa: sembra più comune utilizza Random di Math.random(). Consulta queste best practice .

I commenti e la prima cosa di switch allinterno di do {} while () sono molto confusi per me personalmente. enterLetter() non restituisce mai true o false, e anche la maggior parte dei nuovi programmatori sa che un do / while itera finché la condizione non viene soddisfatta. Sarebbe più utile descrivere cosa rappresenta 0-3 (e se questo fosse codice di produzione o più complesso, un Enum lo renderebbe notevolmente più comprensibile).

Allinterno di enterLetter(), sarebbe utile spostare int emptyPosition = findEmptyPosition(enteredLetters); chiamata al else if in cui è “utilizzato. Non solo impedisce lallocazione della memoria finché non viene usata, ma è anche più leggibile (e puoi rimuovere la variabile emptyPosition se lo desideri e chiamare la funzione allinterno del riferimento , ma questa è semantica).

Lascia un commento

Il tuo indirizzo email non sarà pubblicato. I campi obbligatori sono contrassegnati *