4
\$\begingroup\$

I wrote a Hangman text-based game in Java that needs to include the features that I included in the comments of my code.

In short, the game will ask the user to type a word that they (or a 2nd person) will then guess. The word will be censored by the program. The program will tell the user if their guessed letter is in the word or not, and show the progress of the censored word after each guess. If the user already guessed the letter before, the program will tell the user of that and show their previous guesses without repeating any letters. The program will show the number of attempts at the end.

The code I wrote below works and has all the features I listed. But it seems not optimal and probably with very poor etiquette, as I'm self-taught thus far. Therefore, I'm looking for any advice that will make this code better and to ensure I don't get into any bad habits (I probably already have haha) as I continue to learn Java by myself.

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

}

Sample output of my code for reference if needed:

enter image description here

\$\endgroup\$
2
\$\begingroup\$

Some simple changes:

You create two scanners, one inside the loop and one poorly named at the start. I rename the typedword to input and replace uses of guess with input.

if(repeated == false)

would be better written

if(!repeated)

Similarly I alter other if statements

I would use a Set<String> to store past guesses

I've moved the declaration of times to within the !repeated loop so that its declaration is nearer its use, and limited in scope to its use.

Other declarations have been joined to the setting of the value, and some assignments chained such as the new

String wordToGuess = input.nextLine().toUpperCase();

tempstring has been removed, it is only constructed now when needed

a number of variables have been renamed for more explanatory names.

Final code:

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!");
    }
}
\$\endgroup\$
  • \$\begingroup\$ another suggestion: It might be interesting to store the guess in a char instead of a String, but that changes some minor things in the logic... \$\endgroup\$ – Vogel612 Jul 28 '17 at 10:55
0
\$\begingroup\$

Thanks for sharing your code! It looks pretty good, but(as always) there are some things to fix and look out for:

  1. Creating multiple Scanner instances: Don't. You only need one. Creating more than one just takes up more space. Rather than creating typed word and guess, just create one called input or something like that.

  2. Closing your Scanners: Always do so when you are done using them, or else you will get a "Resource leak" warning. Closing a Scanner makes it so that the Scanner cannot be used again. It's like turning off the lights when you leave the room, there's no point in leaving them on. It's just a waste if you do.

  3. Using == with booleans. Instead of ==, use !, like this:

    if(condition) { //if 'condition' is true.
    

    or

    if(!condition) { //if 'condition' is false
    
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.