patternjavaMinor
Simple calculation in Java
Viewed 0 times
calculationsimplejava
Problem
I am kind of new to Java and I have tried making a simple calculator. This lets you do different calculations in a row until you type done then you get the answer. It works as I expected but I was just wondering if I could get some feedback on it and how I can improve on it or what better approaches I could have taken.
This is my main class:
Then the class with my
This is my main class:
public class Main {
static float Result = 0.0f;
static String input, Iteration;
static float firstInput = 0.0f;
static float secondInput;
static Calculations calculations = new Calculations();
public static void main(String[] args) {
getFirstNumber();
tryCatchMethod();
calculations.sortCalc();
outPutResult();
}
public static void getFirstNumber(){
input = getInput("Enter the first number to work with");
}
public static void getIteration(){
Iteration = getInput("Please enter the number");
try {
secondInput = Float.parseFloat(Iteration);
} catch (NumberFormatException e) {
input = getInput("Please enter a valid number");
tryCatchMethod(); ////recursion, this is for when a valid number is not entered. This will continue happening until a valid number is entered.
}
}
private static void tryCatchMethod(){
try {
firstInput = Float.parseFloat(input);
Result = firstInput;
} catch (NumberFormatException e) {
input = getInput("Please enter a valid number");
tryCatchMethod(); ////recursion, this is for when a valid number is not entered. This will continue happening until a valid number is entered.
}
}
public static void outPutResult(){
System.out.println("the result is :" + Result);
}
static String getInput(String prompt) {
BufferedReader stdin = new BufferedReader(new InputStreamReader(System.in));
System.out.print(prompt);
System.out.flush();
try {
return stdin.readLine();
} catch (Exception e) {
return "Error: " + e.getMessage();
}
}
}Then the class with my
Solution
First things first, you can compare your solution with some of the other calculator implementations in Java on this site. :)
Names in Java
The naming convention used in Java is
You have an inconsistent approach to your field names (
Class inheritance
You may have misunderstood the usage of class inheritance given the extent of having two classes (
First, your
The usual approach for this kind of functionality is to re-think how your arithmetic methods take in two inputs to give you an output. Setting aside the
If you still prefer to have these four methods in a standalone class, give your
Second, your
So, once again, you will not need
In fact, you may also want to consider combining both classes together, and exposing the
Class state vs
The one thing that strikes out as odd to me in your code is the heavy reliance on
From a beginner's point-of-view, I t
Names in Java
The naming convention used in Java is
camelCase for field names, and preferably the singular form for class names. The preference is such that you have instance-s of a Customer class, not Customers.You have an inconsistent approach to your field names (
input, Iteration), and since you are getting started in Java, I will suggest sticking to the naming convention so tha it's easier for you to read and understand other Java code using the same convention, in the long run.Class inheritance
You may have misunderstood the usage of class inheritance given the extent of having two classes (
Multiplication, Calculations) extending another (Main) when they don't share a common... state.First, your
Multiplication class name is ill-advised. When I first looked at it, I thought you were going to create sub-classes for all four arithmetic operators. It turns out that Multiplication implements them as four methods. Furthermore, the only relation between this class and the parent class Main is the sharing of a single static field Result (I'll get to this later).The usual approach for this kind of functionality is to re-think how your arithmetic methods take in two inputs to give you an output. Setting aside the
float-or-double question, you should consider having methods like this:public static float plus(float a, float b) {
return a + b;
}- Method visibility modifier is
publicas it is likely to be used outside the class, not because everything defaults topublic.
staticmethod as the calculation only depends on the two method arguments, and no other class state.
- The use of the two method arguments makes it clear what the inputs are, rather than letting callers of the method guess how they need to pass it in.
- Again, the use of the return type also makes it clear where the output is going - back to the caller of this method.
If you still prefer to have these four methods in a standalone class, give your
Multiplication class a quick rename (CalculatorUtils?) and you can safely - and should - remove the extends Main declaration: it is simply not needed.Second, your
Calculations class looks... messy. It's messy in the sense that:- It's magically responsible for instantiating your
Multiplicationclass,
- It has poorly-named method names, and
- It doesn't require anything from the
Mainclass, other than thestaticmethods.
So, once again, you will not need
extends Main here. Furthermore, when you have the static methods suggested above from the now-renamed CalculatorUtils class, your switch statement should resemble something more like this:public static float compute(String operator) {
// assume that you have a method `getFloatInput()` that returns a float
switch (operator) {
case "+":
return CalculatorUtils.plus(getFloatInput(), getFloatInput());
case "-":
return CalculatorUtils.minus(getFloatInput(), getFloatInput());
case "*":
return CalculatorUtils.multiply(getFloatInput(), getFloatInput());
case "/":
return CalculatorUtils.divide(getFloatInput(), getFloatInput());
default:
throw new IllegalArgumentException("Invalid operator.");
}
}In fact, you may also want to consider combining both classes together, and exposing the
float inputs in the method signature too:// final is used here to prevent sub-classing, since it's not really required
public final class CalculatorUtils {
private CalculatorUtils() {
// private constructor to prevent instantiation, this class
// is meant to be used solely through its static methods
}
// now that we have the compute() method, perhaps we no longer
// need these methods to be accessible outside of this class
private static float plus(float a, float b) {
return a + b;
}
// ...
public static float compute(String operator, float a, float b) {
switch (operator) {
case "+":
return plus(a, b);
case "-":
return minus(a, b);
case "*":
return multiply(a, b);
case "/":
return divide(a, b);
default:
throw new IllegalArgumentException("Invalid operator.");
}
}
}Class state vs
static fieldsThe one thing that strikes out as odd to me in your code is the heavy reliance on
static fields and methods with void return types. This indicates that while you seem to think you need inheritance here (when you don't, see above section), you have forgotten about how to make effective use of class methods to pass information around explicitly, instead choosing to rely on a global state.From a beginner's point-of-view, I t
Code Snippets
public static float plus(float a, float b) {
return a + b;
}public static float compute(String operator) {
// assume that you have a method `getFloatInput()` that returns a float
switch (operator) {
case "+":
return CalculatorUtils.plus(getFloatInput(), getFloatInput());
case "-":
return CalculatorUtils.minus(getFloatInput(), getFloatInput());
case "*":
return CalculatorUtils.multiply(getFloatInput(), getFloatInput());
case "/":
return CalculatorUtils.divide(getFloatInput(), getFloatInput());
default:
throw new IllegalArgumentException("Invalid operator.");
}
}// final is used here to prevent sub-classing, since it's not really required
public final class CalculatorUtils {
private CalculatorUtils() {
// private constructor to prevent instantiation, this class
// is meant to be used solely through its static methods
}
// now that we have the compute() method, perhaps we no longer
// need these methods to be accessible outside of this class
private static float plus(float a, float b) {
return a + b;
}
// ...
public static float compute(String operator, float a, float b) {
switch (operator) {
case "+":
return plus(a, b);
case "-":
return minus(a, b);
case "*":
return multiply(a, b);
case "/":
return divide(a, b);
default:
throw new IllegalArgumentException("Invalid operator.");
}
}
}public void basicAdd(){
Result = Result + secondInput;
}public static float plus(float a, float b) {
return a + b;
}Context
StackExchange Code Review Q#104461, answer score: 5
Revisions (0)
No revisions yet.