patternjavaMinor
Command line Hangman game
Viewed 0 times
commandhangmangameline
Problem
I was wondering if you can give me advice / and your opinion on the following code. I'm aware there are a few things that it doesn't presently deal with that could be enhanced (I don't have time, this is part of an assignment and the basic functionality is complete).
-
Notifying user of their guess count has to be increased, if word chosen from file is greater in length than the guess count (if the guess count is lower than word length, in most cases, user can never win)
-
Ignoring punctuation when selecting words from text file
```
/*
* Game.java
*
* V001
*
* 30.11.2016
*/
import java.io.BufferedReader;
import java.io.FileReader;
import java.io.FileNotFoundException;
import java.io.IOException;
import java.util.Random;
import java.io.InputStreamReader;
public class Game {
// check if args[2] is an int
public static boolean CheckInt(String input) {
boolean valid = true;
for (int i = 0; i 0) {
valid = false;
System.out.println("First argument must be integer");
System.exit(1);
}
}
return valid;
}
public static void main(String[] args) {
BufferedReader reader = null;
if (args.length != 2) {
System.out.println("Please enter two arguments");
System.exit(2);
}
boolean result = CheckInt(args[1]);
// try to open file
try {
reader = new BufferedReader(new FileReader(args[0]));
} catch (FileNotFoundException fnfe) {
System.out.println("Error opening file" + args[0]);
System.exit(3);
}
boolean done = false;
String inputLine = null;
String[] words = null;
int length = 0;
while (!done) {
try {
inputLine = reader.readLine();
} catch (IOException ioe) {
System.out.println("I/O error");
System.exit(4);
}
//end of file
if (inputLine == null) {
done = true;
} else {
String line = inputLine;
String delimter = " ";
words = line.split(delimter);
length = words.length;
}
}
//random number generator to select word from array
Random generator = new
-
Notifying user of their guess count has to be increased, if word chosen from file is greater in length than the guess count (if the guess count is lower than word length, in most cases, user can never win)
-
Ignoring punctuation when selecting words from text file
```
/*
* Game.java
*
* V001
*
* 30.11.2016
*/
import java.io.BufferedReader;
import java.io.FileReader;
import java.io.FileNotFoundException;
import java.io.IOException;
import java.util.Random;
import java.io.InputStreamReader;
public class Game {
// check if args[2] is an int
public static boolean CheckInt(String input) {
boolean valid = true;
for (int i = 0; i 0) {
valid = false;
System.out.println("First argument must be integer");
System.exit(1);
}
}
return valid;
}
public static void main(String[] args) {
BufferedReader reader = null;
if (args.length != 2) {
System.out.println("Please enter two arguments");
System.exit(2);
}
boolean result = CheckInt(args[1]);
// try to open file
try {
reader = new BufferedReader(new FileReader(args[0]));
} catch (FileNotFoundException fnfe) {
System.out.println("Error opening file" + args[0]);
System.exit(3);
}
boolean done = false;
String inputLine = null;
String[] words = null;
int length = 0;
while (!done) {
try {
inputLine = reader.readLine();
} catch (IOException ioe) {
System.out.println("I/O error");
System.exit(4);
}
//end of file
if (inputLine == null) {
done = true;
} else {
String line = inputLine;
String delimter = " ";
words = line.split(delimter);
length = words.length;
}
}
//random number generator to select word from array
Random generator = new
Solution
Having a huge
Let's go through you code:
-
The code parses and validates the command-line arguments. I'd make a separate method for it.
-
It reads something from a file. It's a good candidate for a separate method, too.
-
After that, it generates a random word. I'd make a method
-
After that, the game loop starts. I'd recommend to refactor it into a bunch a small and simple methods (for reading user's input, comparing it to the word she should guess, and checking if the game is over).
In fact, it can be a good idea to go even further and split your code into multiple classes (a class should also do one focused thing). A reasonable way to do it would be to, make the following classes:
There multiple benefits to this approach:
To sum up, I'd recommend to make the code more modular by having a separate classes and methods for each thing that your code needs to do.
main method that does (almost) everything is not a very good idea. It makes your code pretty hard to read. Ideally, a method should do one focused thing. Let's go through you code:
-
The code parses and validates the command-line arguments. I'd make a separate method for it.
-
It reads something from a file. It's a good candidate for a separate method, too.
-
After that, it generates a random word. I'd make a method
getRandomWord to make it more clear and easier to change in the future (if you ever need a change).-
After that, the game loop starts. I'd recommend to refactor it into a bunch a small and simple methods (for reading user's input, comparing it to the word she should guess, and checking if the game is over).
In fact, it can be a good idea to go even further and split your code into multiple classes (a class should also do one focused thing). A reasonable way to do it would be to, make the following classes:
ArgumentParserfor parsing and validating command line arguments.
WordGeneratorfor reading the words from a file and emitting random words.
UserInputReaderfor processing user's input.
- A class that handles the game logic (and just it: no logging, reading input and stuff like that).
- A class for logging (like displaying errors and showing the output to the user. These two could be two separate classes, too).
There multiple benefits to this approach:
- The code becomes more readable.
- It becomes simpler as every method is short and isolated.
- It's easier to change.
- It's easier to test. It's a good habit to cover you code with unit-tests. It's nearly impossible to write automatic modular tests for your code right now. Did you just test it manually?
To sum up, I'd recommend to make the code more modular by having a separate classes and methods for each thing that your code needs to do.
Context
StackExchange Code Review Q#148749, answer score: 5
Revisions (0)
No revisions yet.