Acest cod respectă convențiile standard, nu redundante? Cum îl pot face mai ușor de înțeles pentru alte persoane?
Este un exercițiu din cartea introductivă Java:
Scrieți un joc spânzurător care generează aleatoriu un cuvânt și îi solicită utilizatorului să ghicească câte o literă pe rând. Fiecare literă din cuvânt este afișată ca un asterisc. Când utilizatorul face o ghicire corectă, litera reală este apoi afișată. Când utilizatorul termină o cuvânt, afișați numărul de ratări.
Programul meu:
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; } }
Jurnal a lucrării sale:
(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)
Comentarii
- Ar fi bine dacă atașez un link pentru găzduirea de imagini gratuite cu imaginea schemei de blocuri care ilustrează logica programului meu, deoarece ' nu este evident modul în care funcționează sau ' nu este necesar?
- Diagramele de flux și altele asemenea ar crește Înțelegerea a ceea ce faceți '. Că ' este întotdeauna un lucru bun.
- Cred că ' ați făcut o treabă bună eliminând concedierile și creând metode pentru ao face mai concis. Totuși, este puțin greu de urmat. Poate că mai multe nume de metode descriptive ar ajuta (cea mai bună documentație este codul lizibil!)
- codul dvs. este prea complicat, trebuie să explicați ce se întâmplă, deoarece este dificil pentru un student să citească și să știe exact ce ' se întâmplă. Vă mulțumim.
Răspuns
Numere magice: ar fi mai bine să vă mutați definițiile pentru 0,1, 2,3 într-un câmp enum
sau final
, astfel încât să le puteți face referire cu un nume semnificativ. De exemplu final int LETTER_NOT_IN_WORD = 0
sau enum HangmanGuess { LETTER_NOT_IN_WORD }
Redundanță: logica de bază dintre findEmptyLetters
și inEmptyLetters
este același, căutați un char
într-un char[]
. Ați putea refactura astfel:
/* 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)); }
Un lucru de subliniat este, de asemenea, că în implementarea curentă, findEmptyPosition
va reveni n
unde n
este enteredLetters.length + 1
dacă nu există poziții goale. Nu sunt sigur dacă aceasta este funcționalitatea dorită.
Răspuns
Primul lucru (mic): pare mai obișnuit să utilizați Random
decât Math.random()
. Consultați acest articol despre cele mai bune practici .
Comentariile și schimbarea primului lucru în do {} while () sunt foarte confuze pentru mine personal. enterLetter()
nu returnează niciodată true
sau false
și chiar și majoritatea programatorilor noi știu un do / while iterează până la îndeplinirea condiției. Ar fi mai util să descriem ce reprezintă 0-3 (și dacă acest lucru codul de producție sau mai complex, un Enum l-ar face considerabil mai ușor de înțeles).
În interiorul enterLetter()
, ar fi util să mutați int emptyPosition = findEmptyPosition(enteredLetters);
apel la else if
unde este folosit. Nu numai că împiedică alocarea memoriei până când este folosită, dar este și mai ușor de citit (și puteți elimina variabila emptyPosition
dacă doriți și puteți apela funcția din interiorul referinței , dar asta este semantic).