It is necessary to have consistent and correct (as per style convention) white space. This includes vertical alignment (such as indentation) and spacing between and around particular syntax.
Convention on braces vary, but often braces are encouraged for single statement blocks. One reason is that a later revision may not realise to add braces when adding another statement to the block.
The following is how I might format the code. Note that I am not afraid to add extra blank lines to separate structures. I am not opposed to having three blank lines between methods, though I did not do that here.
import java.util.Arrays;
import java.util.Scanner;
import java.util.Random;
public class Main {
static Scanner input = new Scanner(System.in);
public static void main(String[] args) {
String[] words = {"java", "hello"};
int random = (int)(0 + Math.random() * words.length);
String wordToGuess = words[random];
boolean isGuessed = false;
int counter = 0;
int maxGuess = 7;
char[] chars = new char[wordToGuess.length()];
for (int i = 0; i < chars.length; i++) {
chars[i] = '-';
}
while (!isGuessed && counter < 7) {
System.out.println("Enter your guess: ");
char guess = input.next().charAt(0);
counter++;
maxGuess--;
for (int i = 0; i < wordToGuess.length(); i++) {
if (wordToGuess.charAt(i) == guess) {
chars[i] = guess;
}
}
if (doesContain(chars)) {
for (int j = 0; j < wordToGuess.length(); j++) {
System.out.print(chars[j] + " ");
}
System.out.println("Guess left: " + maxGuess);
} else {
System.out.println("Good job! The word is: ");
for (int j = 0; j < wordToGuess.length(); j++) {
System.out.print(chars[j] + " ");
}
System.out.print("Number of tries: ");
System.out.print(counter);
isGuessed = true;
}
}
}
public static boolean doesContain(char[] chars) {
for (int i = 0; i < chars.length; i++) {
if (chars[i] == '-') {
return true;
}
}
return false;
}
}
This blob concerns me.
String[] words = {"java", "hello"};
int random = (int)(0 + Math.random() * words.length);
String wordToGuess = words[random];
boolean isGuessed = false;
int counter = 0;
int maxGuess = 7;
char[] chars = new char[wordToGuess.length()];
It is best to keep separate what can be kept separate. I took a look at "x depends on y" written as x ⇒ y
and came up with this.
chars ⇒ wordToGuess
wordToGuess ⇒ words
wordToGuess ⇒ random
random ⇒ words
Therefore we can break and reorder these variable declarations like so:
String[] words = {"java", "hello"};
int random = (int)(0 + Math.random() * words.length);
String wordToGuess = words[random];
char[] chars = new char[wordToGuess.length()];
boolean isGuessed = false;
int counter = 0;
int maxGuess = 7;
Then looking down a bit further there is a for-loop which only depends on chars
, so we can pull that up.
String[] words = {"java", "hello"};
int random = (int)(0 + Math.random() * words.length);
String wordToGuess = words[random];
char[] chars = new char[wordToGuess.length()];
for (int i = 0; i < chars.length; i++) {
chars[i] = '-';
}
boolean isGuessed = false;
int counter = 0;
int maxGuess = 7;
There are seven variables in scope (are accessible) for the rest of main
. For each variable one must wonder "does this matter later?". This increases the effort to understand a block of code. Doing a text search I found that words
and random
are unused later. Therefore, lets more closely contain them.
String wordToGuess;
{
String[] words = {"java", "hello"};
int random = (int)(0 + Math.random() * words.length);
wordToGuess = words[random];
}
char[] chars;
{
chars = new char[wordToGuess.length()];
for (int i = 0; i < chars.length; i++) {
chars[i] = '-';
}
}
Now the program is well formatted and just by looking at dependencies we have improved the organisation (we did not have to know what the program does). Further improvements can come from rethinking some of how the program does its task. There seems to be quite a bit happening in the while-loop so I want to consider that first.
Reading the code and doing some string searching, I have determined what the while-loop reads and writes.
Reads & Writes
isGuessed
counter
maxGuess
chars
input
Reads Only
Writes Only
N/A
Lets discover the purpose of each variable which is both read and written. Why these particularly? If a variable is only read it is invariant (unchanging) during the loop. If a variable is only written it has no effect on the loop behaviour.
input
is used to read through stdin
a character at a time, once each iteration.
counter
is increased once per iteration. This is used to print the number of guesses thus far and to terminate the loop if !(counter < 7)
.
maxGuess
is decreased once per iteration. This is used to print the number of guesses remaining.
chars
is updated with the guessed letter once per iteration.
isGuessed
is set to true only if the guessed word is complete. This is used to terminate the loop if !!isGuessed
.
I see a redundancy between counter
and maxGuess
. counter = 7 - maxGuess
or maxGuess = counter + 7
. I think counting down to zero from maxGuess
makes more sense than counting up to 7, so I will replace counter
with maxGuess
. Also I rename maxGuess
to guessesRemaining
and introduce a final variable maxGuesses
assigned to 7.
boolean isGuessed = false;
final int maxGuesses = 7;
int guessesRemaining = maxGuesses;
while (!isGuessed && guessesRemaining > 0) {
System.out.println("Enter your guess: ");
char guess = input.next().charAt(0);
guessesRemaining--;
for (int i = 0; i < wordToGuess.length(); i++) {
if (wordToGuess.charAt(i) == guess) {
chars[i] = guess;
}
}
if (doesContain(chars)) {
for (int j = 0; j < wordToGuess.length(); j++) {
System.out.print(chars[j] + " ");
}
System.out.println("Guess left: " + guessesRemaining);
} else {
System.out.println("Good job! The word is: ");
for (int j = 0; j < wordToGuess.length(); j++) {
System.out.print(chars[j] + " ");
}
System.out.print("Number of tries: ");
System.out.print(maxGuesses - guessesRemaining);
isGuessed = true;
}
}
Final variables cannot be changed, so we know automatically the most the while-loop does with this variable is read it, which it does. Thus we have reduced the number of variables the loop both reads and writes and this can simplify comprehension.
While reading I found the method name doesContain
does not suggest its purpose well. Thus I renamed it to isSolved
. I also thought chars
was too generic of a name so I renamed it to incompleteWord
.
I found '-'
was used as a placeholder letter in both main
and isSolved
. We want to avoid magic literals in our program. A "magic literal" is some literal (such as a string, character, or number) which occurs in one or more places and has an non-obvious and unstated reason for being the value it is. When reading code with magic literals we are not sure what the code does. When updating code with magic literals we are not sure where all we need to make the replacement, especially because there may be different reasons for a particular literal. To fix this we can simply name the literal.
static final char placeholderLetter = '-';
...
incompleteWord[i] = placeholderLetter;
...
if (incompleteWord[i] == placeholderLetter) {
There is much from here we can still do. However, I think this is a good place to stop seeing as this is your first substantial program.
I do not think giving advice or answers on how to implement a program from the abstract is on topic. Rather, this Q&A service is for suggesting improvements to code which is already working and understood. If you want to know how to add some ASCII art to your program then I suggest Stack Overflow.
This is my final version of your program after moving a couple more declarations around.
import java.util.Arrays;
import java.util.Scanner;
import java.util.Random;
public class Main {
static Scanner input = new Scanner(System.in);
static final char placeholderLetter = '-';
static final final int maxGuesses = 7;
static final String[] words = {"java", "hello"};
public static void main(String[] args) {
String wordToGuess;
{
int random = (int)(0 + Math.random() * words.length);
wordToGuess = words[random];
}
char[] incompleteWord;
{
incompleteWord = new char[wordToGuess.length()];
for (int i = 0; i < incompleteWord.length; i++) {
incompleteWord[i] = '-';
}
}
boolean isGuessed = false;
int guessesRemaining = maxGuesses;
while (!isGuessed && guessesRemaining > 0) {
System.out.println("Enter your guess: ");
char guess = input.next().charAt(0);
guessesRemaining--;
for (int i = 0; i < wordToGuess.length(); i++) {
if (wordToGuess.charAt(i) == guess) {
incompleteWord[i] = guess;
}
}
if (isSolved(incompleteWord)) {
for (int j = 0; j < wordToGuess.length(); j++) {
System.out.print(incompleteWord[j] + " ");
}
System.out.println("Guess left: " + guessesRemaining);
} else {
System.out.println("Good job! The word is: ");
for (int j = 0; j < wordToGuess.length(); j++) {
System.out.print(incompleteWord[j] + " ");
}
System.out.print("Number of tries: ");
System.out.print(maxGuesses - guessesRemaining);
isGuessed = true;
}
}
}
public static boolean isSolved(char[] incompleteWord) {
for (int i = 0; i < incompleteWord.length; i++) {
if (incompleteWord[i] == placeholderLetter) {
return true;
}
}
return false;
}
}