Onko tämä koodi tavanomaisten käytäntöjen mukainen, ei tarpeeton? Kuinka voin tehdä siitä ymmärrettävämmän muille?

Se on Java-johdantokirjan harjoitus:

Kirjoita hirsipuu, joka tuottaa satunnaisesti sanan ja kehottaa käyttäjää arvaamaan yhden kirjaimen kerrallaan. Jokainen sanan kirjain näkyy tähdellä. Kun käyttäjä arvaa oikein, varsinainen kirjain näytetään sitten. sana, näytä epäonnistumisten määrä.

Oma ohjelma:

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

Loki työnsä:

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

Kommentit

  • Olisiko hyvä, jos liitän linkin vapaiden kuvien isännöintiin lohkokaaviokuvalla, joka havainnollistaa ohjelmani logiikka, koska se ' ei ole ilmeistä, miten se toimii, tai se ' ei ole välttämätöntä?
  • Vuokaaviot ja vastaavat kasvaisivat th Voit ymmärtää, mitä ' teet. Se ' on aina hyvä asia.
  • Luulen, että olet ' tehnyt hyvää työtä poistamalla irtisanomiset ja luomalla menetelmiä sen tiivistämiseksi. Sitä on kuitenkin vähän vaikea seurata. Ehkä kuvailevien menetelmien nimet auttaisivat (paras dokumentaatio on luettavissa oleva koodi!)
  • koodisi on liian monimutkainen, sinun on selitettävä, mitä tapahtuu, koska opiskelijan on vaikea lukea ja tietää tarkalleen mitä ' tapahtuu. Kiitos.

Vastaa

Maagiset numerot: Olisi parempi siirtää määritelmät arvoksi 0,1, 2,3 kenttiin enum tai final, jotta voit viitata niihin mielekkäällä nimellä. Esimerkiksi final int LETTER_NOT_IN_WORD = 0 tai enum HangmanGuess { LETTER_NOT_IN_WORD }

Redundanssi: Ydinlogiikka findEmptyLetters ja inEmptyLetters on sama, etsit char -kohtaa char[] . Voit refraktoida näin:

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

Yksi asia on myös huomauttaa, että nykyisessä toteutuksessa findEmptyPosition palauttaa n jossa n on enteredLetters.length + 1, jos tyhjiä paikkoja ei ole. En ole varma, onko tämä haluttu toiminto.

Vastaa

Ensimmäinen (pieni) asia: se näyttää yleisemmältä käytä Random kuin Math.random(). Katso tämä parhaiden käytäntöjen viesti.

Kommentit ja vaihdon ensimmäinen asia tekemisen sisällä {}, kun () on minulle henkilökohtaisesti hyvin sekava. enterLetter() ei koskaan palauta true tai false, ja jopa suurin osa uusista ohjelmoijista tietää do / -toiminnon iteroidessaan, kunnes ehto täyttyy. Olisi hyödyllistä kuvata, mitä 0-3 edustaa (ja jos Jos tuotantokoodi olisi monimutkaisempi, Enum tekisi siitä huomattavasti ymmärrettävämmän).

enterLetter(): n sisällä olisi hyödyllistä siirtää int emptyPosition = findEmptyPosition(enteredLetters); kutsu else if -palveluun, missä sitä käytetään. Se ei vain estä muistin jakamista, kunnes sitä käytetään, mutta se on myös luettavampi (ja voit poistaa muuttujan emptyPosition, jos haluat, ja kutsua funktion viitteen sisällä , mutta se on semanttinen).

Vastaa

Sähköpostiosoitettasi ei julkaista. Pakolliset kentät on merkitty *