Volgt deze code de standaardconventies, niet overbodig? Hoe kan ik het begrijpelijker maken voor andere mensen?

Het is een oefening uit het Java-inleidende boek:

Schrijf een galggame die willekeurig een woord genereert en de gebruiker vraagt om één letter tegelijk te raden. Elke letter in het woord wordt weergegeven als een asterisk. Wanneer de gebruiker een goede schatting maakt, wordt de eigenlijke letter weergegeven. word, geef het aantal missers weer.

Mijn 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 van zijn werk:

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

Reacties

  • Zou het goed zijn als ik een link bijvoeg op gratis image-hosting met een afbeelding van een blokschema ter illustratie logica van mijn programma, omdat het ' niet duidelijk is hoe het werkt of het ' is niet nodig?
  • Stroomdiagrammen en dergelijke zouden th toenemen De begrijpelijkheid van wat u ' doet. Dat ' is altijd een goede zaak.
  • Ik denk dat je ' goed werk hebt verricht door overtolligheden te verwijderen en methoden om het beknopter te maken. Het is echter een beetje moeilijk te volgen. Misschien zouden meer beschrijvende methodenamen helpen (de beste documentatie is leesbare code!)
  • je code is te gecompliceerd, je moet uitleggen wat er aan de hand is, aangezien het moeilijk is voor een student om te lezen en precies te weten wat ' s gebeurt. Dank je.

Antwoord

Magische getallen: het zou beter zijn om je definities te verplaatsen voor 0,1, 2,3 in een enum of final velden zodat u ernaar kunt verwijzen met een betekenisvolle naam. Bijvoorbeeld final int LETTER_NOT_IN_WORD = 0 of enum HangmanGuess { LETTER_NOT_IN_WORD }

Redundantie: de kernlogica tussen findEmptyLetters en inEmptyLetters is hetzelfde, je zoekt een char in een char[] . Je zou als volgt kunnen refactoren:

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

Een ding om op te merken is ook dat in de huidige implementatie findEmptyPosition retourneert n waarbij n de enteredLetters.length + 1 is als er geen lege posities zijn. Ik “weet niet zeker of dit de gewenste functionaliteit is.

Antwoord

Eerste (kleine) ding: het lijkt gebruikelijker om gebruik Random dan Math.random(). Zie dit artikel met praktische tips .

Het commentaar en het eerste punt binnen do {} while () is erg verwarrend voor mij persoonlijk. enterLetter() retourneert nooit true of false, en zelfs de meeste nieuwe programmeurs weten dat een do / while itereert totdat aan de voorwaarde is voldaan. Het zou handiger zijn om te beschrijven wat 0-3 vertegenwoordigen (en of dit waren productiecode of complexer, zou een Enum het aanzienlijk begrijpelijker maken).

Binnen enterLetter() zou het nuttig zijn om de int emptyPosition = findEmptyPosition(enteredLetters); roep de else if aan waar deze wordt gebruikt. Het voorkomt niet alleen het toewijzen van geheugen totdat het “is gebruikt, maar het is ook beter leesbaar (en je zou de emptyPosition variabele kunnen verwijderen als je wilde en de functie binnen de referentie oproepen , maar dat is semantisch).

Geef een reactie

Het e-mailadres wordt niet gepubliceerd. Vereiste velden zijn gemarkeerd met *