patternjavaMinor
Dr. Nim game in Java
Viewed 0 times
gamenimjava
Problem
This video should explain how the game works, but in simple form, it's a game where you enter a number between 1 and 3, This number is than removed from 12, in doing so the computer will then pick a number between 1-3 based on what number you have picked, if you choose to remove 1 marble it should say that it has removed 3 marbles. This works because there are 3 lots of 4 in 12. And the maths tells it to remove what number you have chosen from 4. Because there are 3 lots of 4, you can never win, which is the whole point of the game.
Is there any way I could improve on the code?
```
import java.util.Scanner;
public class Main {
public static int input;
public static int marble = 12;
public static int comFirst = (int) (Math.random() * 3 + 1);
@SuppressWarnings("resource")
public static void main(String[] args) throws InterruptedException {
System.out.println("There Are 12 Marbles, You Can Only Remove Up To 3 Marbles At A Time");
System.out.println("Your Goal Is To Remove The Last Marble, Marble 1");
System.out.println("To Remove A Marble Either Enter 1, 2 or 3");
System.out.println("");
while (true) {
if (marble == 1) {
System.out.println("There Is " + marble + " Marble Remaining");
} else {
System.out.println("There Are " + marble + " Marbles Remaining");
}
Scanner scanInput = new Scanner(System.in);
input = scanInput.nextInt();
if (input > 3 || input < 1) {
System.out.println("You Can Only Remove 3 Marbles At A Time");
} else {
marble = marble - input;
Thread.sleep(250);
int cpuAnswer = 4 - input;
if (cpuAnswer == 1) {
System.out.println("The Computer Has Removed " + cpuAnswer + "
Is there any way I could improve on the code?
```
import java.util.Scanner;
public class Main {
public static int input;
public static int marble = 12;
public static int comFirst = (int) (Math.random() * 3 + 1);
@SuppressWarnings("resource")
public static void main(String[] args) throws InterruptedException {
System.out.println("There Are 12 Marbles, You Can Only Remove Up To 3 Marbles At A Time");
System.out.println("Your Goal Is To Remove The Last Marble, Marble 1");
System.out.println("To Remove A Marble Either Enter 1, 2 or 3");
System.out.println("");
while (true) {
if (marble == 1) {
System.out.println("There Is " + marble + " Marble Remaining");
} else {
System.out.println("There Are " + marble + " Marbles Remaining");
}
Scanner scanInput = new Scanner(System.in);
input = scanInput.nextInt();
if (input > 3 || input < 1) {
System.out.println("You Can Only Remove 3 Marbles At A Time");
} else {
marble = marble - input;
Thread.sleep(250);
int cpuAnswer = 4 - input;
if (cpuAnswer == 1) {
System.out.println("The Computer Has Removed " + cpuAnswer + "
Solution
Break early instead of nested if
The above can be rewritten as a simple if statement with continue, this prevent the indention from creeper up at the side of the screen.
Placing the scanner outside the loop
Scanners use internal buffering to buffer its inputstream, this mean they may consume large amounts, instead of only the part they need for the the concurrent operation. When placing the scanner outside the loop, this also allows for the player to put in the moves much quicker after each other, an still having the program pick up every move.
Place
This also allows you to get rid of
Proper english doesn't have all words title case
(all println statements)
SOme people have problems with reading sentences with every word uppercase, use the english grammar rules, make only names and the first word of a sentence starting with a uppercase.
Check the state of
You should check the state of
Unused variables
At the moment, you have 1 unused variable. Either you will be using this variable for a future expansion, or this variable is left while you were working on the application.
You have won case triggers after the computer move
This case triggers after the computer has done its move. If you really planning on the player to win sometime, you should move this case before the computer makes a move.
Win cases too strict
While with the current implementation of your application, all wins are perfect and ending with exactly 0. If you update your implementation of the computer or player in the future, this may pose a problem in the future.
The final result would look like this:
Splitting the main method
By splitting the main method you archieve better code reuse.
@TheCoffeeCup has written an excellent answer for this part of the code.
if (input > 3 || input < 1) {
System.out.println("You Can Only Remove 3 Marbles At A Time");
} else {
....
}The above can be rewritten as a simple if statement with continue, this prevent the indention from creeper up at the side of the screen.
if (input > 3 || input < 1) {
System.out.println("You Can Only Remove 3 Marbles At A Time");
continue;
}
....Placing the scanner outside the loop
Scanners use internal buffering to buffer its inputstream, this mean they may consume large amounts, instead of only the part they need for the the concurrent operation. When placing the scanner outside the loop, this also allows for the player to put in the moves much quicker after each other, an still having the program pick up every move.
Place
Scanner scanInput = new Scanner(System.in); before while(true){, and wrap the scanner in a try with resources like this:try(Scanner scanInput = new Scanner(System.in)){
while(true) {
....
}
}This also allows you to get rid of
@SuppressWarnings("resource").Proper english doesn't have all words title case
(all println statements)
SOme people have problems with reading sentences with every word uppercase, use the english grammar rules, make only names and the first word of a sentence starting with a uppercase.
Check the state of
scanInput.hasNextInt()You should check the state of
scanInput.hasNextInt() before every number is get, this method can return false if the program is accessed by the command line and the command line return EOF, this can be triggered with ctrl + D on linux. The best way to do this is replacing the true in your while loop with the check.Unused variables
public static int comFirst = (int) (Math.random() * 3 + 1);At the moment, you have 1 unused variable. Either you will be using this variable for a future expansion, or this variable is left while you were working on the application.
You have won case triggers after the computer move
This case triggers after the computer has done its move. If you really planning on the player to win sometime, you should move this case before the computer makes a move.
Win cases too strict
if (marble - cpuAnswer == 0) {
if (marble - input == 0) {While with the current implementation of your application, all wins are perfect and ending with exactly 0. If you update your implementation of the computer or player in the future, this may pose a problem in the future.
if (marble - cpuAnswer =< 0) {
if (marble - input =< 0) {The final result would look like this:
import java.util.Scanner;
public class Main {
public static int input;
public static int marble = 12;
public static void main(String[] args) throws InterruptedException {
System.out.println("There Are 12 Marbles, You Can Only Remove Up To 3 Marbles At A Time");
System.out.println("Your Goal Is To Remove The Last Marble, Marble 1");
System.out.println("To Remove A Marble Either Enter 1, 2 or 3");
System.out.println("");
try (Scanner scanInput = new Scanner(System.in)) {
while (scanInput.hasNextInput()) {
if (marble == 1) {
System.out.println("There Is " + marble + " Marble Remaining");
} else {
System.out.println("There Are " + marble + " Marbles Remaining");
}
input = scanInput.nextInt();
if (input > 3 || input < 1) {
System.out.println("You Can Only Remove 3 Marbles At A Time");
continue;
}
marble = marble - input;
if (marble - input =< 0) {
System.out.println("There Are No Marbles Remaining, You Have Won!!");
return;
}
Thread.sleep(250);
int cpuAnswer = 4 - input;
if (cpuAnswer == 1) {
System.out.println("The Computer Has Removed " + cpuAnswer + " Marble");
} else {
System.out.println("The Computer Has Removed " + cpuAnswer + " Marbles");
}
if (marble - cpuAnswer =< 0) {
System.out.println("There Are No Marbles Remaining, The Computer Has Won!!");
System.out.println("The Computer Has Won");
return;
}
marble = marble - cpuAnswer;
}
}
}
}Splitting the main method
By splitting the main method you archieve better code reuse.
@TheCoffeeCup has written an excellent answer for this part of the code.
Code Snippets
if (input > 3 || input < 1) {
System.out.println("You Can Only Remove 3 Marbles At A Time");
} else {
....
}if (input > 3 || input < 1) {
System.out.println("You Can Only Remove 3 Marbles At A Time");
continue;
}
....try(Scanner scanInput = new Scanner(System.in)){
while(true) {
....
}
}public static int comFirst = (int) (Math.random() * 3 + 1);if (marble - cpuAnswer == 0) {
if (marble - input == 0) {Context
StackExchange Code Review Q#118617, answer score: 6
Revisions (0)
No revisions yet.