Napisałem tekstową grę Hangman w Javie, która musi zawierać funkcje, które umieściłem w komentarzach do mojego kodu .
Krótko mówiąc, gra poprosi użytkownika o wpisanie słowa, które on (lub druga osoba) zgadnie. Program ocenzuruje słowo. Program poinformuje użytkownika, czy jego odgadnięta litera jest w słowie, czy nie, i pokaże postęp ocenzurowanego słowa po każdym odgadnięciu. Jeśli użytkownik odgadł już literę, program poinformuje o tym użytkownika i pokaże jego poprzednie domysły bez powtarzania liter. Program pokaże liczbę prób na końcu.
Kod, który napisałem poniżej, działa i ma wszystkie funkcje, które wymieniłem. Wydaje się jednak, że nie jest to optymalne i prawdopodobnie ma bardzo złą etykietę, ponieważ do tej pory jestem samoukiem. Dlatego szukam porady, która poprawi ten kod i zapewni, że nie wpadnę w żadne złe nawyki ( Prawdopodobnie mam już haha), ponieważ kontynuuję samodzielną naukę języka Java.
//Simple Hangman game where user types a word, program stores it in all CAPS for easier user readability and censors the word (i.e *****) //User then guesses one letter at a time until the entire word is guessed. Program will inform the user if the guess is in the word, and show the progress of the word after each guess. //If the guessed letter is in the word, program will print out the # of times the letter is in the word. //Program will store and print out # of guesses (attempts) needed to guess the word at the end of the program. //If user tries to duplicate a previous guess, program will inform user of that and show previous guesses by user. Attempt count will not go up for duplicate guesses. //When the program shows previous guesses by the user (using a string), it cannot contain duplicate letters. (i.e: if user guesses "s" twice, "s" will still only show up once in the string) //StackOverFlow readers: This program works as intended, but as a self-taught beginner coder, I need assistance on optimal coding style (less lines the better) and good coding principles/etiquette //I definitely think there are much better ways to code this, but I cannot think of any more (as you probably noticed, this is v3, which has more features and yet similar amount of lines as version 1 haha) //All and any help is appreciated! Thank you :D import java.util.*; public class HangmanGameV3 { public static void main(String [] args){ //Initialize all the variables used here String storedword; char[] charstring; int length; char[] censor; int attempts=0; StringBuilder pastguesses = new StringBuilder(); //String Builder to add and print out previous guesses Scanner typedword = new Scanner(System.in); System.out.println("Enter your word to guess: "); storedword = typedword.nextLine(); storedword = storedword.toUpperCase(); //stores the word and changes it to all caps length = storedword.length(); charstring = storedword.toCharArray(); //creates char array of string //creates and prints an array of chars with the same length as string censor = storedword.toCharArray(); System.out.println("Your secret word is: "); for (int index = 0; index < length; index++){ censor[index] = "*"; } //Main loop to take guesses (is this while loop the ideal loop here? while (String.valueOf(censor).equals(storedword)== false){ //Initialize all variables in loop char charguess; String tempword; String tempstring; boolean correct = false; //required for if loops below/lets the user know if the letter is in the word or not int times = 0; //number of times a letter is in the word boolean repeated = false; //check if user guessed the same letter twice //prints the censored secret word for(int a= 0; a < length; a++){ System.out.print(censor[a]); } System.out.println(); //asks user for guess, then stores guess in Char charguess and String tempstring Scanner guess = new Scanner(System.in); System.out.println("Type your guess: "); tempword = guess.next(); charguess = tempword.charAt(0); //gets char data from scanner pastguesses.append(charguess); //adds guess to previous guess string tempstring = pastguesses.toString(); //checks if user already guessed the letter previously if (tempstring.lastIndexOf(charguess, tempstring.length() -2 ) != -1){ System.out.println("You already guessed this letter! Guess again. Your previous guesses were: "); pastguesses.deleteCharAt(tempstring.length()-1); System.out.println(tempstring.substring(0, tempstring.length()-1)); repeated = true; } //if the guess is not a duplicated guess, checks if the guessed letter is in the word if (repeated == false){ for (int index = 0; index < length; index++){ if(charstring[index] == Character.toUpperCase(charguess)) { censor[index] = Character.toUpperCase(charguess); //replaces * with guessed letter in caps correct = true; times++; } } if(correct == true){ System.out.println("The letter " + charguess + " is in the secret word! There are " + times +" " + charguess + " "s in the word. Revealing the letter(s): "); } else if (correct == false){ System.out.println("Sorry, the letter is not in the word. Your secret word: "); } System.out.println(); } attempts++; } System.out.println("You guessed the entire word "+ storedword.toUpperCase() + " correctly! It took you " + attempts + " attempts!"); //typedword.close(); //StackOverFlow readers: is this necessary? Not sure how to use .close() }
Przykładowe wyjście mojego kodu w razie potrzeby:
Odpowiedź
Kilka prostych zmian:
Tworzysz dwa skanery , jeden wewnątrz pętli, a drugi o złej nazwie na początku. Zmieniam nazwę typedword
na input
i zamień zastosowania guess
na input
.
if(repeated == false)
byłoby lepiej napisane
if(!repeated)
Podobnie zmieniam inne, jeśli instrukcje
Użyłbym Set<String>
do przechowywania wcześniejszych domysłów
Miałem ruch d deklaracja times
w obrębie !repeated loop
, tak aby jej deklaracja była bliższa jej użycia i ograniczona w zakresie do jej użycia.
Inne deklaracje zostały dołączone do ustawienia wartości, a niektóre przypisania zostały połączone w łańcuch, na przykład nowy
String wordToGuess = input.nextLine().toUpperCase();
tempstring
został usunięty, jest konstruowany tylko wtedy, gdy jest potrzebny.
Wiele zmiennych zostało przemianowanych na bardziej objaśniające nazwy.
Ostateczny kod:
import java.util.HashSet; import java.util.Scanner; import java.util.Set; public class HangmanGameV3 { public static void main(String[] args) { int attempts = 0; Set<String> previousGuesses = new HashSet<>(); Scanner input = new Scanner(System.in); System.out.println("Enter your word to guess: "); String wordToGuess = input.nextLine().toUpperCase(); int length = wordToGuess.length(); char[] wordToGuessChars = wordToGuess.toCharArray(); //creates char array of string //creates and prints an array of chars with the same length as string char[] censor = wordToGuess.toCharArray(); System.out.println("Your secret word is: "); for (int index = 0; index < length; index++) { censor[index] = "*"; } //Main loop to take guesses (is this while loop the ideal loop here? while (!String.valueOf(censor).equals(wordToGuess)) { //Initialize all variables in loop boolean correct = false; //required for if loops below/lets the user know if the letter is in the word or not boolean repeated = false; //check if user guessed the same letter twice //prints the censored secret word for (int a = 0; a < length; a++) { System.out.print(censor[a]); } System.out.println(); //asks user for guess, then stores guess in Char charguess and String tempstring System.out.println("Type your guess: "); String currentGuess = input.next().toUpperCase().substring(0, 1); char currentGuessChar = currentGuess.charAt(0); //gets char data from scanner //checks if user already guessed the letter previously if (previousGuesses.contains(currentGuess)) { System.out.println("You already guessed this letter! Guess again. Your previous guesses were: "); System.out.println(previousGuesses.stream().reduce("", String::concat)); repeated = true; } previousGuesses.add(currentGuess); //if the guess is not a duplicated guess, checks if the guessed letter is in the word if (!repeated) { int times = 0; //number of times a letter is in the word for (int index = 0; index < length; index++) { if (wordToGuessChars[index] == currentGuessChar) { censor[index] = currentGuessChar; //replaces * with guessed letter in caps correct = true; times++; } } if (correct) { System.out.println("The letter " + currentGuessChar + " is in the secret word! There are " + times + " " + currentGuessChar + " "s in the word. Revealing the letter(s): "); } else { System.out.println("Sorry, the letter is not in the word. Your secret word: "); } System.out.println(); } attempts++; } System.out.println("You guessed the entire word " + wordToGuess.toUpperCase() + " correctly! It took you " + attempts + " attempts!"); } }
Komentarze
Odpowiedź
Dziękujemy za udostępnienie kodu! Wygląda całkiem nieźle, ale (jak zawsze) jest kilka rzeczy do naprawienia i zwrócenia uwagi:
-
Tworzenie wielu instancji
Scanner
: Nie „t. Potrzebujesz tylko jednego. Utworzenie więcej niż jednego zajmuje po prostu więcej miejsca. Zamiast tworzyćtyped word
iguess
, po prostu utwórz jeden o nazwieinput
lub coś w tym stylu. -
Zamykanie
Scanners
: Zawsze rób więc kiedy skończysz ich używać, albo otrzymasz ostrzeżenie o „wycieku zasobów”. ZamknięcieScanner
powoduje, żeScanner
nie można ich użyć ponownie. To tak, jakbyś wyłączał światło, gdy wychodzisz z pokoju, nie ma sensu zostawiać ich włączonych. To tylko strata, jeśli to zrobisz. -
Używanie
==
z wartościami logicznymi. Zamiast==
użyj!
, na przykład:if(condition) { //if "condition" is true.
lub
if(!condition) { //if "condition" is false
char
zamiast String, ale to zmienia kilka drobnych rzeczy w logice …