Okay, let's start by talking about formatting. This code is formatted "correctly", in that a compiler can work with it. However, the formatting standards you employ will be difficult to work with for just about any other java developer that exists. Readability is one of the most important features of any codebase. If code can't be read and understood by the reader, it is dangerous to try to modify it because the results are unclear.
In idiomatic java:
- variables are declared one per line
- there is only one statement per line
- open curly braces are on the same line, not a newline
- there is a newline after an open curly brace
- close curly braces are on a newline, not the same line
- there is no whitespace before a ;
- there is a blank line before the start of a method
- there is whitespace between a control flow keyword (
for
, if
, etc) and the open paren.
- method names start with a lowercase letter (they use camelCase).
- class names start with a capital letter, but the remaining letters of the word or acronym are lowercase. This is easier to read. Prefer
Mash
to MASH
, even if it stands for something.
- code is indented properly. Each level of indentation should be either 2 or 4 whitespace characters, not tabs. Any time a line ends with an open curly, the lines below it get one additional level of indentation. Any time a line ends with a close curly, the lines below give back that level of indentation.
- curly braces are always used, even when the compiler doesn't require them. This prevents a class of error and also makes it easier to read the code.
- lines are limited to a reasonable length, often 120 characters.
If your organization does not have a coding standards guideline, I would suggest working from either Oracle's coding standards or Google's coding standards. Alternately, if you're using an IDE, it will certainly have an auto-format feature. Use that. It can probably be configured to run on save and format as you work.
Separate, but related - whitespace and formatting should be employed consistently. When I look at code that uses whitespace inconsistently, I immediately doubt whether attention to detail has paid in any other part of the code.
With these changes, the code looks like:
public class Mash {
int max;
static int ch;
char type;
char lvl;
String sch;
int ans;
int x;
int y;
int c = 0;
int n;
long startTime;
long stopTime;
Scanner sc = new Scanner(System.in);
Random r = new Random();
public void intro() throws InterruptedException {
System.out.println(
"Hi! I am M.A.S.H, your personal Mental Arithmetic Speed-up Helping tool.\nI am developed to help people increase their mental arithmetic speed ");
System.out.println(
"which in turn will help to increase your Problem solving speed.\n\nThere are several options to choose from and hopefully, more will be added in future updates!\nSo Go ahead and let me help you.");
start();
}
public void input() {
ch = sc.nextInt();
}
public void start() throws InterruptedException {
System.out.println(
"Great! Let's Start...\n\nLet's begin with ASMR. No! not that obnoxious sound you make while chewing your food, where are your manners brat ?\n\nAnyways here ASMR means Addition, Subtraction, Multiplication or Random Mix.\n\nSelect the one which you would like to practice :");
System.out.println("\n1. Addition\n2. Subtraction \n3. Multiplication\n4. Random Mix");
System.out.print("\nEnter Choice : ");
input();
switch (ch) {
case 1:
type = 'a';
break;
case 2:
type = 's';
break;
case 3:
type = 'm';
break;
case 4:
type = 'r';
break;
default:
System.out.println("Invalid Choice");
System.out.println("Default : Random Mix");
type = 'r';
}
System.out.println("\nWhat do you want to practice :");
System.out.println("1. One Digit Problems\n2. Two Digit Problems\n3. Arbitary Problems");
System.out.print("\nEnter Choice : ");
input();
switch (ch) {
case 1:
lvl = '1';
break;
case 2:
lvl = '2';
break;
case 3:
lvl = 'a';
break;
default:
System.out.println("Invalid Choice");
System.out.println("Default : Arbitary Problems");
lvl = 'a';
}
System.out.println("\nSelect Mode :");
System.out.println(
"1. Streak : Try to complete as many problems as you can without any mistakes, No time limit\n2. Timed : Try to complete a fixed number of questions and see the time you took at the end");
System.out.print("\nEnter Choice : ");
input();
switch (ch) {
case 1:
streak();
break;
case 2:
timed();
break;
}
}
public void streak() {
System.out.print("Alright Let's Start!");
check();
for (int i = 0; i < 10;) {
x = rndm();
y = rndm();
if (type == 'a') {
add();
} else if (type == 's') {
subtract();
} else if (type == 'm') {
multiply();
} else {
int cal = r.nextInt(3);
if (cal == 0) {
add();
} else if (cal == 1) {
multiply();
} else {
subtract();
}
}
input();
if (ans == ch) {
System.out.println("\nCorrect");
c++;
} else {
System.out.println("\nWrong");
break;
}
if (c % 10 == 0) {
System.out.println(c + " in a row, Keep Going!");
}
if (c == 100) {
System.out.println("\nCentury!!!!");
}
}
System.out.println("Streak Of : " + c);
finish();
}
public void timed() throws InterruptedException {
System.out.println("How many questions do you want to solve ?");
System.out.println("1. 5\n2. 10\n3. 20\n4. Custom");
input();
switch (ch) {
case 1:
n = 5;
break;
case 2:
n = 10;
break;
case 3:
n = 20;
break;
case 4:
System.out.println("\nHow many Questions do you want to solve ?");
n = sc.nextInt();
break;
default:
System.out.println("Invalid Choice\nDefault : 10");
n = 10;
}
check();
System.out.println("\nAll Set To Start");
System.out.println("3");
Thread.sleep(1000);
System.out.println("2");
Thread.sleep(1000);
System.out.println("1");
Thread.sleep(1000);
System.out.println("Go!!!");
startTime = System.currentTimeMillis();
for (int i = 0; i < n; i++) {
x = rndm();
y = rndm();
if (type == 'a') {
add();
} else if (type == 's') {
subtract();
} else if (type == 'm') {
multiply();
} else {
int sal = r.nextInt(3);
if (sal == 0) {
add();
} else if (sal == 1) {
multiply();
} else {
subtract();
}
}
input();
if (ans == ch) {
c++;
}
}
stopTime = System.currentTimeMillis();
long react = stopTime - startTime;
long temp = react / n;
System.out.println("\nTotal Number of Questions : " + n);
System.out.println("Number of Correct Answers : " + c);
System.out.println("Total Time Taken : " + (react / 1000) + " seconds and " + (react % 1000) + " milliseconds");
System.out.println(
"Average Time per Question : " + (temp / 1000) + " seconds and " + (temp % 1000) + " milliseconds");
finish();
}
void check() {
if (lvl == '1') {
max = 11;
} else {
max = 21;
}
}
int rndm() {
int a = r.nextInt(max);
if (lvl == '2' && a < 10) {
a = a + 10;
}
return (a);
}
void add() {
ans = x + y;
System.out.print(x + " + " + y + "=");
}
void subtract() {
ans = x - y;
System.out.print(x + " - " + y + "=");
}
void multiply() {
ans = x * y;
System.out.print(x + " X " + y + "=");
}
void finish() {
sc.close();
}
public static void main(String args[]) throws InterruptedException {
Mash obj = new Mash();
System.out.println("1. Start\n2. About M.A.S.H");
obj.input();
if (ch == 1) {
obj.start();
} else {
obj.intro();
}
}
}
Next up, access modifiers. In this class, they are used inconsistently. A good rule of thumb when programming is to restrict access as much as possible. Properties and methods should be as visible as is necessary, but no more. It's much easier to understand code when scope is limited. As written, readers of this class are left wondering what other classes in the same package need to be able to call check
, and what other classes in other packages need to be able to call start
and streak
.
In this class, everything should be private. Nothing in this class should be called from outside it. That means all the instance variables and methods (except main
) should have private
in front of them, not nothing or public
.
When I do this, I see a warning pop up that the variable sch
is never used. That's because when it has default access, the compiler thinks we intend it to be used, even if it isn't right now. When we make it private, the compiler realizes it is a bug. So I'm tossing that variable right out.
Now that I'm looking at variables, I see a bunch with very unhelpful names. int c
means nothing to me. Variable names should clearly describe the value they hold, and they should not be abbreviations, because not everyone will understand the abbreviation. answer
is much better than ans
, because a non-native speaker can look up answer
in a dictionary. The same applies for methods. rndm
is much worse than random()
.
When a variable's value has a unit, include that in the variable name. startTimeMillis
is better than startTime
.
It is common to lay out methods in a file in the approximate order they will be used. Typically the entry point (constructor, main method, public methods) are at the top of the file. Private methods are at the bottom. Statics are typically before non-statics.
Now the code looks like:
public class Mash {
private static int userChoice;
private int maximumNumber;
private char arithmeticType;
private char difficultyLevel;
private int answer;
private int firstNumber;
private int secondNumber;
private int numberOfCorrectAnswers = 0;
private int numberOfQuestionsToSolve;
private long startTimeMillis;
private long stopTimeMillis;
private Scanner scanner = new Scanner(System.in);
private Random random = new Random();
public static void main(String args[]) throws InterruptedException {
Mash mash = new Mash();
System.out.println("1. Start\n2. About M.A.S.H");
mash.input();
if (userChoice == 1) {
mash.start();
} else {
mash.intro();
}
}
private void intro() throws InterruptedException {
System.out.println(
"Hi! I am M.A.S.H, your personal Mental Arithmetic Speed-up Helping tool.\nI am developed to help people increase their mental arithmetic speed ");
System.out.println(
"which in turn will help to increase your Problem solving speed.\n\nThere are several options to choose from and hopefully, more will be added in future updates!\nSo Go ahead and let me help you.");
start();
}
private void input() {
userChoice = scanner.nextInt();
}
private void start() throws InterruptedException {
System.out.println(
"Great! Let's Start...\n\nLet's begin with ASMR. No! not that obnoxious sound you make while chewing your food, where are your manners brat ?\n\nAnyways here ASMR means Addition, Subtraction, Multiplication or Random Mix.\n\nSelect the one which you would like to practice :");
System.out.println("\n1. Addition\n2. Subtraction \n3. Multiplication\n4. Random Mix");
System.out.print("\nEnter Choice : ");
input();
switch (userChoice) {
case 1:
arithmeticType = 'a';
break;
case 2:
arithmeticType = 's';
break;
case 3:
arithmeticType = 'm';
break;
case 4:
arithmeticType = 'r';
break;
default:
System.out.println("Invalid Choice");
System.out.println("Default : Random Mix");
arithmeticType = 'r';
}
System.out.println("\nWhat do you want to practice :");
System.out.println("1. One Digit Problems\n2. Two Digit Problems\n3. Arbitary Problems");
System.out.print("\nEnter Choice : ");
input();
switch (userChoice) {
case 1:
difficultyLevel = '1';
break;
case 2:
difficultyLevel = '2';
break;
case 3:
difficultyLevel = 'a';
break;
default:
System.out.println("Invalid Choice");
System.out.println("Default : Arbitary Problems");
difficultyLevel = 'a';
}
System.out.println("\nSelect Mode :");
System.out.println(
"1. Streak : Try to complete as many problems as you can without any mistakes, No time limit\n2. Timed : Try to complete a fixed number of questions and see the time you took at the end");
System.out.print("\nEnter Choice : ");
input();
switch (userChoice) {
case 1:
streak();
break;
case 2:
timed();
break;
}
}
private void streak() {
System.out.print("Alright Let's Start!");
check();
for (int i = 0; i < 10;) {
firstNumber = random();
secondNumber = random();
if (arithmeticType == 'a') {
add();
} else if (arithmeticType == 's') {
subtract();
} else if (arithmeticType == 'm') {
multiply();
} else {
int cal = random.nextInt(3);
if (cal == 0) {
add();
} else if (cal == 1) {
multiply();
} else {
subtract();
}
}
input();
if (answer == userChoice) {
System.out.println("\nCorrect");
numberOfCorrectAnswers++;
} else {
System.out.println("\nWrong");
break;
}
if (numberOfCorrectAnswers % 10 == 0) {
System.out.println(numberOfCorrectAnswers + " in a row, Keep Going!");
}
if (numberOfCorrectAnswers == 100) {
System.out.println("\nCentury!!!!");
}
}
System.out.println("Streak Of : " + numberOfCorrectAnswers);
finish();
}
private void timed() throws InterruptedException {
System.out.println("How many questions do you want to solve ?");
System.out.println("1. 5\n2. 10\n3. 20\n4. Custom");
input();
switch (userChoice) {
case 1:
numberOfQuestionsToSolve = 5;
break;
case 2:
numberOfQuestionsToSolve = 10;
break;
case 3:
numberOfQuestionsToSolve = 20;
break;
case 4:
System.out.println("\nHow many Questions do you want to solve ?");
numberOfQuestionsToSolve = scanner.nextInt();
break;
default:
System.out.println("Invalid Choice\nDefault : 10");
numberOfQuestionsToSolve = 10;
}
check();
System.out.println("\nAll Set To Start");
System.out.println("3");
Thread.sleep(1000);
System.out.println("2");
Thread.sleep(1000);
System.out.println("1");
Thread.sleep(1000);
System.out.println("Go!!!");
startTimeMillis = System.currentTimeMillis();
for (int i = 0; i < numberOfQuestionsToSolve; i++) {
firstNumber = random();
secondNumber = random();
if (arithmeticType == 'a') {
add();
} else if (arithmeticType == 's') {
subtract();
} else if (arithmeticType == 'm') {
multiply();
} else {
int sal = random.nextInt(3);
if (sal == 0) {
add();
} else if (sal == 1) {
multiply();
} else {
subtract();
}
}
input();
if (answer == userChoice) {
numberOfCorrectAnswers++;
}
}
stopTimeMillis = System.currentTimeMillis();
long react = stopTimeMillis - startTimeMillis;
long temp = react / numberOfQuestionsToSolve;
System.out.println("\nTotal Number of Questions : " + numberOfQuestionsToSolve);
System.out.println("Number of Correct Answers : " + numberOfCorrectAnswers);
System.out.println("Total Time Taken : " + (react / 1000) + " seconds and " + (react % 1000) + " milliseconds");
System.out.println(
"Average Time per Question : " + (temp / 1000) + " seconds and " + (temp % 1000) + " milliseconds");
finish();
}
private void check() {
if (difficultyLevel == '1') {
maximumNumber = 11;
} else {
maximumNumber = 21;
}
}
private int random() {
int a = random.nextInt(maximumNumber);
if (difficultyLevel == '2' && a < 10) {
a = a + 10;
}
return (a);
}
private void add() {
answer = firstNumber + secondNumber;
System.out.print(firstNumber + " + " + secondNumber + "=");
}
private void subtract() {
answer = firstNumber - secondNumber;
System.out.print(firstNumber + " - " + secondNumber + "=");
}
private void multiply() {
answer = firstNumber * secondNumber;
System.out.print(firstNumber + " X " + secondNumber + "=");
}
private void finish() {
scanner.close();
}
}
Next, looking at the top of the class, I see that the code is checking the user choice, and based on that going to start()
or intro()
.. but then intro()
calls start()
. In that case, I'd expect the start()
call to be outside the if()
check inside main. Then intro()
doesn't need to throw an exception.
While we're poking, it's often easier to read println
statements if each output line is in its own statement, rather than bunching them with \n
.
public static void main(String args[]) throws InterruptedException {
Mash mash = new Mash();
System.out.println("1. Start\n2. About M.A.S.H");
mash.input();
if (userChoice == 2) {
mash.intro();
}
mash.start();
}
private void intro() {
System.out.println("Hi! I am M.A.S.H, your personal Mental Arithmetic Speed-up Helping tool.");
System.out.println("I am developed to help people increase their mental arithmetic speed ");
System.out.println( "which in turn will help to increase your Problem solving speed.");
System.out.println();
System.out.println(
"There are several options to choose from and hopefully, more will be added in future updates!");
System.out.println("So Go ahead and let me help you.");
}
I also see that the input()
method is reading from the console, and then assigning that value to an instance variable. I would say that this method is not a useful abstraction, because it's more clear to read scanner.nextInt()
than to read input()
.
The other issue here is that every instance variable is something else that the reader has to keep track of for the whole class. We want it to be as easy for the reader as possible. Since this variable is only ever used in the method where input
is called, we can make it a local variable in each method instead.
In order to do this, we need to make the scanner static
, because it's called from main
. Instead, I would suggest moving the initial prompt out of main.
The code now looks like:
public class Mash {
private int maximumNumber;
private char arithmeticType;
private char difficultyLevel;
private int answer;
private int firstNumber;
private int secondNumber;
private int numberOfCorrectAnswers = 0;
private int numberOfQuestionsToSolve;
private long startTimeMillis;
private long stopTimeMillis;
private Scanner scanner = new Scanner(System.in);
private Random random = new Random();
public static void main(String args[]) throws InterruptedException {
Mash mash = new Mash();
mash.promptIntro();
}
private void promptIntro() throws InterruptedException {
System.out.println("1. Start\n2. About M.A.S.H");
int userChoice = scanner.nextInt();
if (userChoice == 2) {
intro();
}
start();
}
private void intro() {
System.out.println("Hi! I am M.A.S.H, your personal Mental Arithmetic Speed-up Helping tool.");
System.out.println("I am developed to help people increase their mental arithmetic speed ");
System.out.println( "which in turn will help to increase your Problem solving speed.");
System.out.println();
System.out.println(
"There are several options to choose from and hopefully, more will be added in future updates!");
System.out.println("So Go ahead and let me help you.");
}
private void start() throws InterruptedException {
System.out.println(
"Great! Let's Start...\n\nLet's begin with ASMR. No! not that obnoxious sound you make while chewing your food, where are your manners brat ?\n\nAnyways here ASMR means Addition, Subtraction, Multiplication or Random Mix.\n\nSelect the one which you would like to practice :");
System.out.println("\n1. Addition\n2. Subtraction \n3. Multiplication\n4. Random Mix");
System.out.print("\nEnter Choice : ");
int userChoice = scanner.nextInt();
switch (userChoice) {
case 1:
arithmeticType = 'a';
break;
case 2:
arithmeticType = 's';
break;
case 3:
arithmeticType = 'm';
break;
case 4:
arithmeticType = 'r';
break;
default:
System.out.println("Invalid Choice");
System.out.println("Default : Random Mix");
arithmeticType = 'r';
}
System.out.println("\nWhat do you want to practice :");
System.out.println("1. One Digit Problems\n2. Two Digit Problems\n3. Arbitary Problems");
System.out.print("\nEnter Choice : ");
int userDifficultyChoice = scanner.nextInt();
switch (userDifficultyChoice) {
case 1:
difficultyLevel = '1';
break;
case 2:
difficultyLevel = '2';
break;
case 3:
difficultyLevel = 'a';
break;
default:
System.out.println("Invalid Choice");
System.out.println("Default : Arbitary Problems");
difficultyLevel = 'a';
}
System.out.println("\nSelect Mode :");
System.out.println(
"1. Streak : Try to complete as many problems as you can without any mistakes, No time limit\n2. Timed : Try to complete a fixed number of questions and see the time you took at the end");
System.out.print("\nEnter Choice : ");
int userModeChoice = scanner.nextInt();
switch (userModeChoice) {
case 1:
streak();
break;
case 2:
timed();
break;
}
}
private void streak() {
System.out.print("Alright Let's Start!");
check();
for (int i = 0; i < 10;) {
firstNumber = random();
secondNumber = random();
if (arithmeticType == 'a') {
add();
} else if (arithmeticType == 's') {
subtract();
} else if (arithmeticType == 'm') {
multiply();
} else {
int cal = random.nextInt(3);
if (cal == 0) {
add();
} else if (cal == 1) {
multiply();
} else {
subtract();
}
}
int userChoice = scanner.nextInt();
if (answer == userChoice) {
System.out.println("\nCorrect");
numberOfCorrectAnswers++;
} else {
System.out.println("\nWrong");
break;
}
if (numberOfCorrectAnswers % 10 == 0) {
System.out.println(numberOfCorrectAnswers + " in a row, Keep Going!");
}
if (numberOfCorrectAnswers == 100) {
System.out.println("\nCentury!!!!");
}
}
System.out.println("Streak Of : " + numberOfCorrectAnswers);
finish();
}
private void timed() throws InterruptedException {
System.out.println("How many questions do you want to solve ?");
System.out.println("1. 5\n2. 10\n3. 20\n4. Custom");
int userQuestionsChoice = scanner.nextInt();
switch (userQuestionsChoice) {
case 1:
numberOfQuestionsToSolve = 5;
break;
case 2:
numberOfQuestionsToSolve = 10;
break;
case 3:
numberOfQuestionsToSolve = 20;
break;
case 4:
System.out.println("\nHow many Questions do you want to solve ?");
numberOfQuestionsToSolve = scanner.nextInt();
break;
default:
System.out.println("Invalid Choice\nDefault : 10");
numberOfQuestionsToSolve = 10;
}
check();
System.out.println("\nAll Set To Start");
System.out.println("3");
Thread.sleep(1000);
System.out.println("2");
Thread.sleep(1000);
System.out.println("1");
Thread.sleep(1000);
System.out.println("Go!!!");
startTimeMillis = System.currentTimeMillis();
for (int i = 0; i < numberOfQuestionsToSolve; i++) {
firstNumber = random();
secondNumber = random();
if (arithmeticType == 'a') {
add();
} else if (arithmeticType == 's') {
subtract();
} else if (arithmeticType == 'm') {
multiply();
} else {
int sal = random.nextInt(3);
if (sal == 0) {
add();
} else if (sal == 1) {
multiply();
} else {
subtract();
}
}
int userAnswer = scanner.nextInt();
if (answer == userAnswer) {
numberOfCorrectAnswers++;
}
}
stopTimeMillis = System.currentTimeMillis();
long react = stopTimeMillis - startTimeMillis;
long temp = react / numberOfQuestionsToSolve;
System.out.println("\nTotal Number of Questions : " + numberOfQuestionsToSolve);
System.out.println("Number of Correct Answers : " + numberOfCorrectAnswers);
System.out.println("Total Time Taken : " + (react / 1000) + " seconds and " + (react % 1000) + " milliseconds");
System.out.println(
"Average Time per Question : " + (temp / 1000) + " seconds and " + (temp % 1000) + " milliseconds");
finish();
}
private void check() {
if (difficultyLevel == '1') {
maximumNumber = 11;
} else {
maximumNumber = 21;
}
}
private int random() {
int a = random.nextInt(maximumNumber);
if (difficultyLevel == '2' && a < 10) {
a = a + 10;
}
return (a);
}
private void add() {
answer = firstNumber + secondNumber;
System.out.print(firstNumber + " + " + secondNumber + "=");
}
private void subtract() {
answer = firstNumber - secondNumber;
System.out.print(firstNumber + " - " + secondNumber + "=");
}
private void multiply() {
answer = firstNumber * secondNumber;
System.out.print(firstNumber + " X " + secondNumber + "=");
}
private void finish() {
scanner.close();
}
}
There's plenty more to play with here, but it's my bedtime. Perhaps someone else will pick up where I left off.