Problem
I’ve created a second ‘calculator-like’ program, this time using the JOptionPane rather than typing in the console. At the moment the code looks to have a lot of repetition so I’m looking to simplify it; any advice and suggestions would be great!
import javax.swing.JOptionPane;
public class SaveCalc {
public static void main(String[] args) {
//Text prompts to collect data
String carCost = JOptionPane.showInputDialog("What is the total cost of the car?");
String insCost = JOptionPane.showInputDialog("What is the yearly cost of the insurance?");
String manCost = JOptionPane.showInputDialog("How much would you like to save for maintenance?");
String saveMonth = JOptionPane.showInputDialog("How much will you be saving each month?");
//Converts string inputs to Doubles
double carCostP = Double.parseDouble(carCost);
double insCostP = Double.parseDouble(insCost);
double manCostP = Double.parseDouble(manCost);
double saveMonthP = Double.parseDouble(saveMonth);
//Calculates the total cost and the months to save
double costTotal = carCostP + insCostP + manCostP;
double months = costTotal / saveMonthP;
//Converts the months value to an integer
int monthsP = (int) months;
System.out.printf
("With a saving of u00A3%5.2f each month and a total cost of u00A3%5.2f it will take "
+ monthsP
+ " months to save up.",saveMonthP,costTotal);
}
}
Solution
Do not include types in variable names
carCostP
^
Remove that P
(probably stands for a number type?) the compiler takes care of types for you.
Avoid so many variables
You can avoid those four variables altogether, you may just convert to double as soon as you read the vars, like this:
double carCost = Double.parseDouble(
JOptionPane.showInputDialog("What is the total cost of the car?"));
Additionally to Caridorcs answer you should avoid using variables for the only reason than changing the type of a result (like you do it with months
).
Just cast it to an other type, or better round your months
variable, when you need it.
You don’t check if the user’s input can be parsed to double
, which will cause an exception when you parse. To check if your input is numerical, you can use the following function, which uses a regex :
public static boolean isNumeric(String str)
{
return str.matches("-?\d+(\.\d+)?"); //match a number with optional '-' and decimal.
}
This example comes from this SO’s answer
This way, you can validate your input instead of letting the application crash if I enter non-numerical characters as an input (note that this regex will accept “.” and “-” as valid characters).
You need to loop on your input until it is valid, so you could do something like this :
String carCost = JOptionPane.showInputDialog("What is the total cost of the car?");
while(!isNumeric(carCost)){
carCost = JOptionPane.showInputDialog("Please enter a valid car cost : ");
}
Afterwards, you could extract this logic into a method, let’s call it getDoubleInput(String paneText, String invalidInputText)
private String getDoubleInput(String paneText, String invalidInputText){
String input = JOptionPane.showInputDialog(paneText);
while(!isNumeric(input)){
input = JOptionPane.showInputDialog(invalidInputText);
}
return input;
}
Then you could, as @Caridorc’s pointed, skip the String
variable in your code and use this format :
double carCost = Double.parseDouble(getDoubleInput("What is the total cost of the car?","Please enter a valid car cost : "));
double insCostP = Double.parseDouble(getDoubleInput("What is the yearly cost of the insurance?", "Please enter a valid yearly cost of insurance : "));
//...
If you dislike have 2 String
parameter to the method, you could use a generic invalid input message such as : “Please enter a valid input : “. Then the method would look like this :
private String getDoubleInput(String paneText){
String input = JOptionPane.showInputDialog(paneText);
while(!isNumeric(input)){
input = JOptionPane.showInputDialog("Please enter a valid input : ");
}
return input;
}
You wouldn’t need the second parameter in the method call :
double carCost = Double.parseDouble(getDoubleInput("What is the total cost of the car?"));
A small but possibly important point (or so I think ;)):
.. this time using the JOptionPane rather than typing in the console…
And then you have this:
System.out.printf
("With a saving of u00A3%5.2f each month and a total cost of u00A3%5.2f it will take "
+ monthsP
+ " months to save up.",saveMonthP,costTotal);
Formatting issue aside (that’s a non-conventional way of alignment…?), since you have opted (pun unintended) to ask the user using a GUI dialog box, you should ideally present the answer back as a dialog box too. Otherwise, users may be hunting where the answers are.
Oh, and about that printf(...)
usage… since you are using it, you should use a placeholder for your monthsP
variable as well.
Don’t use floating point numbers to represent monetary amounts. Prefer BigDecimal
instead.
Casting a double
to an int
truncates the number, that is it drops all the digits after the point. So 9.999 becomes 9. Whereas you should round up.
double months = costTotal / saveMonthP;
//Converts the months value to an integer
int monthsP = (int) months;
Suppose monthsP
turned out 9.001
. This means at the end of 9 months you’ll still come up short (even if by a little), and you’ll have to save for one more month.
You may find it useful to also report the amount that would be needed to be saved in the last month. So that the user might consider his options accordingly. Is that amount something he can come up with in the previous month instead? That he can borrow from friends and family? Or should he consider increasing the monthly saving amount a little to get the car a month earlier? May be he could decrease the saving amount a little to spread the saving amounts evenly among all the months? So on and so forth…