patternjavaModerate
Mortgage calculator
Viewed 0 times
calculatormortgagestackoverflow
Problem
I feel that my app could be more efficient but I am not sure what else I can do. I feel like I could be reusing more. If you could give me some tips on how I could improve this, I would really appreciate it.
```
package com.mortgagecalc;
import java.text.DecimalFormat;
public class Calc {
int term; //how long the loan is
float interestRate; // loan interest rate
float principal; // loan amount
float monthlyPayment; // monthly payment
private int period = 12; // 12 months in a year
//Amortization Variables
int n = 360;
double i = 0.0575;
double
public class Main {
/**
* @param args
* @throws InterruptedException
*/
public static void main(String[] args) throws InterruptedException {
//creating a new instance of my Calc class
Calc calc = new Calc();
// setting the variables
calc.term = 30;
calc.interestRate = (float) 0.0575;
calc.principal = (float) 200000.00;
calc.monthlyPayment = 0;
//format currency correctly
DecimalFormat df = new java.text.DecimalFormat("$,###.00");
//Method call to calc class
calc.DoWork();
//out put
System.out.println("\n__________________________________________________________________________________________________________");
System.out.println("\nPrincipal Amount: " + df.format (calc.principal ));
System.out.println("interest rate: " + calc.interestRate);
System.out.println("Term of loan(number of years): " + calc.term);
System.out.println("Monthly payment is: " + df.format (calc.monthlyPayment));
System.out.println("__________________________________________________________________________________________________________\n");
Thread.sleep(600);
calc.DoWorkAmortization();
}
}```
package com.mortgagecalc;
import java.text.DecimalFormat;
public class Calc {
int term; //how long the loan is
float interestRate; // loan interest rate
float principal; // loan amount
float monthlyPayment; // monthly payment
private int period = 12; // 12 months in a year
//Amortization Variables
int n = 360;
double i = 0.0575;
double
Solution
-
I believe you'd be better off with having term, interestRate,... as privates, and provide those in the Calc constructor.
-
The name DoWork does not convey the task the function performs; a better name could be CalculatePayment. Also, the "do" in "DoWork" suggests that either the function has a side effect or takes a lot of time to compute. The same for DoWorkAmortization, it prints stuff and sleeps; could be PrintAmortizations. Calc class name should also be mor
telling.
-
But the act of conveying information to the user, using a specific way (System.out) and sleeping does not belong to the Calc class. It is better to leave only the calculation responsibility to Calc and the rest to the program.
-
But while you have that function print to the user, then at least catch the InterruptedException and act on it in the function and not pass it further.
-
DoWork does not have to declare InterruptedException as it will not be thrown there.
-
Use intermediate values for subexpressions in DoWork to make the code more readable.
Those are my 2¢.
EDIT: Removed 4. and 5., as InterruptedException should be propagated in this case.
I believe you'd be better off with having term, interestRate,... as privates, and provide those in the Calc constructor.
-
The name DoWork does not convey the task the function performs; a better name could be CalculatePayment. Also, the "do" in "DoWork" suggests that either the function has a side effect or takes a lot of time to compute. The same for DoWorkAmortization, it prints stuff and sleeps; could be PrintAmortizations. Calc class name should also be mor
telling.
-
But the act of conveying information to the user, using a specific way (System.out) and sleeping does not belong to the Calc class. It is better to leave only the calculation responsibility to Calc and the rest to the program.
-
But while you have that function print to the user, then at least catch the InterruptedException and act on it in the function and not pass it further.
-
DoWork does not have to declare InterruptedException as it will not be thrown there.
-
Use intermediate values for subexpressions in DoWork to make the code more readable.
Those are my 2¢.
EDIT: Removed 4. and 5., as InterruptedException should be propagated in this case.
Context
StackExchange Code Review Q#9680, answer score: 11
Revisions (0)
No revisions yet.