Ce code suit-il les conventions standard, nest-il pas redondant? Comment puis-je le rendre plus compréhensible pour les autres?

Cest un exercice du livre dintroduction à Java:

Rédiger un jeu du pendu qui génère un mot au hasard et invite lutilisateur à deviner une lettre à la fois. Chaque lettre du mot est affichée sous la forme dun astérisque. Lorsque lutilisateur fait une estimation correcte, la lettre réelle saffiche. Lorsque lutilisateur termine un mot, affiche le nombre déchecs.

Mon programme:

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

Journal de son travail:

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

Commentaires

  • Serait-il bon de joindre un lien sur lhébergement dimages libres avec limage du schéma de blocs illustrant logique de mon programme, car ' nest pas évident comment cela fonctionne ou ' nest pas nécessaire?
  • Les organigrammes et autres augmenteraient e La compréhension de ce que vous ' faites. ' est toujours une bonne chose.
  • Je pense que vous ' avez fait du bon travail en supprimant les redondances et en créant méthodes pour le rendre plus concis. Cest un peu difficile à suivre, cependant. Peut-être que des noms de méthodes plus descriptifs seraient utiles (la meilleure documentation est un code lisible!)
  • votre code est trop compliqué, vous devez expliquer ce qui se passe, car il est difficile pour un étudiant de lire et de savoir exactement ce qui se passe ' se passe. Merci.

Réponse

Magic Numbers: Il serait préférable de déplacer vos définitions pour 0,1, 2,3 dans un champ enum ou final afin que vous puissiez les référencer avec un nom significatif. Par exemple final int LETTER_NOT_IN_WORD = 0 ou enum HangmanGuess { LETTER_NOT_IN_WORD }

Redondance: la logique de base entre findEmptyLetters et inEmptyLetters est le même, vous recherchez un char dans un char[] . Vous pouvez refactoriser comme ceci:

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

Une chose à souligner également est que dans limplémentation actuelle, findEmptyPosition renverra nn est le enteredLetters.length + 1 sil ny a pas de positions vides. Je ne sais pas si cest la fonctionnalité souhaitée.

Réponse

Première (petite) chose: il semble plus courant de utilisez Random que Math.random(). Consultez ce message sur les bonnes pratiques .

Les commentaires et la première chose à changer dans le do {} while () sont très déroutants pour moi personnellement. enterLetter() ne renvoie jamais true ou false, et même la plupart des nouveaux programmeurs connaissent une itération do / while jusquà ce que la condition soit remplie. Il serait plus utile de décrire ce que 0-3 représente (et si cela étaient du code de production ou plus complexe, un Enum le rendrait considérablement plus compréhensible).

À lintérieur de enterLetter(), il serait utile de déplacer le int emptyPosition = findEmptyPosition(enteredLetters); appel au else if où il « est utilisé. Non seulement cela empêche dallouer la mémoire jusquà ce quelle soit utilisée, mais il est également plus lisible (et vous pouvez supprimer la variable emptyPosition si vous le souhaitez et appeler la fonction à lintérieur de la référence , mais cest sémantique).

Laisser un commentaire

Votre adresse e-mail ne sera pas publiée. Les champs obligatoires sont indiqués avec *