Følger denne kode standardkonventioner, ikke overflødig? Hvordan kan jeg gøre det mere forståeligt for andre mennesker?
Det er en øvelse fra Java-introduktionsbogen:
Skriv en hangman-spil, der tilfældigt genererer et ord og beder brugeren om at gætte et bogstav ad gangen. Hvert bogstav i ordet vises som en stjerne. Når brugeren foretager et korrekt gæt, vises det aktuelle bogstav. Når brugeren er færdig med et ord, vis antallet af savner.
Mit 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; } }
Log af sit arbejde:
(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)
Kommentarer
- Ville det være godt, hvis jeg vedhæfter link til gratis billedhosting med billede af blokskema, der illustrerer logik i mit program, fordi det ' ikke er indlysende, hvordan det fungerer, eller det ' ikke er nødvendigt?
- Flowcharts og lignende ville øge th forståeligheden af, hvad du ' laver. At ' altid er en god ting.
- Jeg synes, du ' har gjort et godt stykke arbejde med at fjerne afskedigelser og skabe metoder til at gøre det mere kortfattet. Det er dog lidt svært at følge. Måske ville mere beskrivende metodenavne hjælpe (den bedste dokumentation er læsbar kode!)
- din kode er for kompliceret, du skal forklare, hvad der foregår, da det er svært for en studerende at læse og vide nøjagtigt hvad ' sker. Tak.
Svar
Magiske tal: Det ville være bedre at flytte dine definitioner til 0,1, 2,3 i et enum
eller final
felter, så du kan henvise til dem med et meningsfuldt navn. For eksempel final int LETTER_NOT_IN_WORD = 0
eller enum HangmanGuess { LETTER_NOT_IN_WORD }
Redundans: Kernelogikken mellem findEmptyLetters
og inEmptyLetters
er det samme, du leder efter en char
i en char[]
. Du kan reflektere sådan:
/* 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)); }
En ting at påpege er også, at findEmptyPosition
i den nuværende implementering returnerer n
hvor n
er enteredLetters.length + 1
hvis der ikke er tomme positioner. Jeg er ikke sikker på, om dette er den ønskede funktionalitet.
Svar
Første (lille) ting: det virker mere almindeligt at brug Random
end Math.random()
. Se dette indlæg med bedste praksis .
Kommentarerne og skift af den første ting inden for do {} mens () er meget forvirrende for mig personligt. enterLetter()
returnerer aldrig true
eller false
, og selv de fleste nye programmører kender et do / mens iterates indtil betingelsen er opfyldt. Det ville være mere nyttigt at beskrive, hvad 0-3 repræsenterer (og hvis dette var produktionskode eller mere kompleks, ville Enum gøre det betydeligt mere forståeligt).
Inde i enterLetter()
, ville det være nyttigt at flytte int emptyPosition = findEmptyPosition(enteredLetters);
kalder til else if
hvor det bruges. Ikke kun forhindrer det at allokere hukommelsen, indtil den bruges, men den er også mere læselig (og du kan fjerne emptyPosition
-variablen, hvis du vil og kalde funktionen inde i referencen , men det er semantisk.