patternjavaModerate
Multithreaded HiLo game
Viewed 0 times
hilogamemultithreaded
Problem
For my CS class I have to write a HiLo game using multithreading. I am brand new to multithreading and not sure how to best implement it. The program below works, but I am wondering if there is a better way to do it. When it is run the user will input an
```
import java.util.Random;
import java.util.Scanner;
class Game implements Runnable {
private static long time;
private long timer;
private static long gameTime;
public Game(int n){
gameTime=n;
}
public void run() {
time=System.currentTimeMillis();
while(true){
timer=(System.currentTimeMillis()-time)/1000;
if(timer>=gameTime){
System.out.println("Oops! Time is up - try again.");
break;
}
}
}
}
public class Hilo {
public static void main(String[] args) {
if (args.length!=1){
System.err.println("Must enter time");
}
Random rand = new Random();
int max=100;
int min=1;
int number=rand.nextInt((max-min)+1)+min;
int gameTime=Integer.parseInt(args[0]);
System.out.println("Welcome to HiLo!");
System.out.println("You have "+gameTime+" seconds to guess a number between 1 and 100.");
Thread clock1 = new Thread(new Game(gameTime));
clock1.start();
while(clock1.isAlive()==true){
System.out.println(">");
Scanner sc = new Scanner(System.in);
int input = sc.nextInt();
if(clock1.isAlive()==true&&input==number){
System.out.println("You Win!");
break;
}else if(clock1.isAlive()==true&&inputnumber){
System.out.println("Lower!");
}
}
}
int, which is the amount of time that they will have to guess the correct number. If the timer runs out the game will end. I am not supposed to use the Timer object but instead use the System.currentTimeMillis(). ```
import java.util.Random;
import java.util.Scanner;
class Game implements Runnable {
private static long time;
private long timer;
private static long gameTime;
public Game(int n){
gameTime=n;
}
public void run() {
time=System.currentTimeMillis();
while(true){
timer=(System.currentTimeMillis()-time)/1000;
if(timer>=gameTime){
System.out.println("Oops! Time is up - try again.");
break;
}
}
}
}
public class Hilo {
public static void main(String[] args) {
if (args.length!=1){
System.err.println("Must enter time");
}
Random rand = new Random();
int max=100;
int min=1;
int number=rand.nextInt((max-min)+1)+min;
int gameTime=Integer.parseInt(args[0]);
System.out.println("Welcome to HiLo!");
System.out.println("You have "+gameTime+" seconds to guess a number between 1 and 100.");
Thread clock1 = new Thread(new Game(gameTime));
clock1.start();
while(clock1.isAlive()==true){
System.out.println(">");
Scanner sc = new Scanner(System.in);
int input = sc.nextInt();
if(clock1.isAlive()==true&&input==number){
System.out.println("You Win!");
break;
}else if(clock1.isAlive()==true&&inputnumber){
System.out.println("Lower!");
}
}
}
Solution
Spacing
First of all, yourcodeispackedverytightly, try using spaces a bit more. This is preferred:
Static variables
I see absolutely no need to have these as
Run and
So here you have a repeating loop that does more or less absolutely nothing as fast as it can. As you already know how long you should wait, use
Using
Scanner resource
This line should give you a compiler warning because you're not closing the scanner. Consider using a try-with-resources statement.
You only need to create the scanner once, when you start the program. Then you can close it safely at the end of your program.
if, if, if...
Do you see any common things there?
First of all, yourcodeispackedverytightly, try using spaces a bit more. This is preferred:
timer = (System.currentTimeMillis() - time) / 1000;
int number = rand.nextInt((max - min) + 1) + min;Static variables
private static long time;
private static long gameTime;I see absolutely no need to have these as
static. As it is right now, your code would get problematic if you instantiated one Game, waited for a while, and then created another game. Each Game should have it's own time and gameTime values.Run and
while (true)public void run() {
time=System.currentTimeMillis();
while(true){
timer=(System.currentTimeMillis()-time)/1000;
if(timer>=gameTime){
System.out.println("Oops! Time is up - try again.");
break;
}
}
}So here you have a repeating loop that does more or less absolutely nothing as fast as it can. As you already know how long you should wait, use
Thread.sleep(delay) instead.==truewhile(clock1.isAlive()==true){Using
==true is redundant, simply this is enough:while (clock1.isAlive()) {Scanner resource
Scanner sc = new Scanner(System.in);This line should give you a compiler warning because you're not closing the scanner. Consider using a try-with-resources statement.
You only need to create the scanner once, when you start the program. Then you can close it safely at the end of your program.
if, if, if...
if(clock1.isAlive()==true&&input==number){
System.out.println("You Win!");
break;
}else if(clock1.isAlive()==true&&inputnumber){
System.out.println("Lower!");
}Do you see any common things there?
clock1.isAlive() is being checked on every if. Sure, it is possible that it is alive in the first check and then dies before the second if, but that is very unlikely and we're talking about microseconds of difference there. Do the clock1.isAlive check once, and then check compare with the number.if (clock1.isAlive()) {
if (input == number) {
System.out.println("You Win!");
break;
} else if(input number) {
System.out.println("Lower!");
}
}Code Snippets
timer = (System.currentTimeMillis() - time) / 1000;
int number = rand.nextInt((max - min) + 1) + min;private static long time;
private static long gameTime;public void run() {
time=System.currentTimeMillis();
while(true){
timer=(System.currentTimeMillis()-time)/1000;
if(timer>=gameTime){
System.out.println("Oops! Time is up - try again.");
break;
}
}
}while(clock1.isAlive()==true){while (clock1.isAlive()) {Context
StackExchange Code Review Q#64034, answer score: 11
Revisions (0)
No revisions yet.