Ez a kód megfelel a szokásos konvencióknak, nem felesleges? Hogyan tehetem érthetőbbé más emberek számára?

Ez a Java bevezető könyvének gyakorlata:

Írjon egy hóhér játék, amely véletlenszerűen generál egy szót, és arra kéri a felhasználót, hogy találgasson egy-egy betűt. A szó minden betűje csillagként jelenik meg. Amikor a felhasználó helyesen tippel, akkor a tényleges betű jelenik meg. Amikor a felhasználó befejezi a szó, jelenítse meg a hiányzások számát.

Saját program:

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

Napló munkája:

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

Megjegyzések

  • Jó lenne, ha csatolnék egy linket a szabad kép tárolásához blokk-séma képével, amely szemlélteti a programom logikája, mert ' nem nyilvánvaló, hogy működik, vagy ' nem szükséges?
  • A folyamatábrák és hasonlók növelnék a th-t Megértése annak, amit ' csinál. Ez ' mindig jó dolog.
  • Szerintem ' jó munkát végzett az elbocsátások eltávolításával és a módszerek tömörebbé tételéhez. Kicsit nehéz azonban követni. Talán a leíróbb metódusnevek segíthetnek (a legjobb dokumentáció az olvasható kód!)
  • a kódod túl bonyolult, meg kell magyaráznod, mi folyik itt, mivel a hallgatónak nehéz elolvasnia és pontosan tudni, hogy mi történik ' s történik. Köszönöm.

Válasz

Mágikus számok: Jobb lenne, ha a definíciókat 0,1-re mozgatnánk, 2,3 <

vagy final mezőkbe, hogy értelmes névvel hivatkozhasson rájuk. Például final int LETTER_NOT_IN_WORD = 0 vagy enum HangmanGuess { LETTER_NOT_IN_WORD }

Redundancia: A fő logika a findEmptyLetters és inEmptyLetters megegyezik, egy char t keres egy char[] . Így refrakcionálhat:

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

Egy dologra is felhívni a figyelmet, hogy a jelenlegi megvalósításban findEmptyPosition visszaadja n ahol n a enteredLetters.length + 1, ha nincsenek üres pozíciók. Nem vagyok biztos benne, hogy ez a kívánt funkció.

Válasz

Első (apró) dolog: úgy tűnik, hogy gyakoribb használja a Random -t, mint a Math.random() -t. Lásd ezt a bevált gyakorlatot . / p>

A megjegyzések és a kapcsoló első dolga a do {} belsejében, míg a () személy szerint nagyon zavaró. enterLetter() soha nem ad vissza true vagy false, és még a legtöbb új programozó is ismeri a do / műveletet, amíg az ismétlődik, amíg a feltétel nem teljesül. Hasznosabb lenne leírni, hogy a 0-3 mit jelent (és ha ez ha termelési kód vagy bonyolultabb lenne, egy Enum lényegesen érthetőbbé tenné).

A enterLetter() oldalon hasznos lenne a int emptyPosition = findEmptyPosition(enteredLetters); hívás a else if helyre, ahol használta. Nem csak megakadályozza a memória lefoglalását, amíg nem lesz használva, hanem olvashatóbb is (és eltávolíthatja a emptyPosition változót, ha akarja, és meghívhatja a függvényt a hivatkozásban. , de ez “szemantikus”.

Vélemény, hozzászólás?

Az email címet nem tesszük közzé. A kötelező mezőket * karakterrel jelöltük