# Mortgage calculator

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.

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 a = 200000.00; double r = (1+i/12); DecimalFormat df = new java.text.DecimalFormat("$,###.00");

//Method to calculate the payment.
void DoWork() throws InterruptedException
{

//Payment calculation
monthlyPayment = (float) (principal * Math.pow(1 + interestRate / period, term * period) * (interestRate / period) / (Math.pow(1 + interestRate / period, term * period) - 1));

}//Method to calculate Amortization

void DoWorkAmortization() throws InterruptedException
{

int number = 1;
double monthlyPayments  = a * ( r - 1 ) / ( 1 - Math.pow(r,-n));
System.out.println ("Month \t\t Payment \t\0\0\0\0 Interest Paid \t\0\0\0\0 Principal Paid \t\0\0\0\0 Remaining Balance");
System.out.println("__________________________________________________________________________________________________________");

for ( number = 1; number <= 360; number++)
{

double interest = a * ( i/12 );
double principal = monthlyPayments - interest ;
double balance = a - principal;
a = balance;

System.out.println ("\0\0"+ number + "\t\t" + df.format(monthlyPayments)+"\t\t" + df.format(interest)+"\t\t\t"+ df.format(principal)+ "\t\t\t" +df.format(balance));

if (number % 12 == 0){

}

}

}
}
• Note that you don't need to write (float) 0.0575, just 0.0575F. Mar 4, 2012 at 12:54

1. I believe you'd be better off with having term, interestRate,... as privates, and provide those in the Calc constructor.

2. 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.

3. 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.

4. 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.

5. DoWork does not have to declare InterruptedException as it will not be thrown there.

6. 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.

• Thank you for your tips :) I was having trouble with just putting the calculations for the Amortization in the Calc class... I am not sure how to pass this. Again thank you for your review. Mar 4, 2012 at 1:51
• You could create a class having getInterest(), getPrincipal() and getBalance() and return it's instances from a CalculateStateOnDay(int day). Mar 4, 2012 at 1:55
• ahh so its more of a getter / setter ? can i do this with in Calc class ? Im going to rename that class btw lol Mar 4, 2012 at 2:03
• interest, principal and balance are all related to a specific day, and as such should not be inside Calc - which is not bound to a specific day. What I'm suggesting is to return them in instances of some class InterestData, which has getters for the three. Those are not really getters from the Calc class. Mar 4, 2012 at 2:07
• But in this simple Calc exercise, it would be easier to have functions getInterest(day), getPrincipal(day) and getBalance(day) in Calc, so you don't have a InterestData class with all properties and no operations Mar 4, 2012 at 2:11
1. Standard naming for Java functions uses a lower case for the first letter - for example instead of DoWorkAmortization it would be doWorkAmortization.

2. In DoWorkAmortization, the declaration of the variable number should occur at the top of the for loop:

for (int number = 1; number <= 360; number++)

rather than four statements above the loop. It's one thing to use a vague variable name as a loop index, but separating the declaration from the use for no reason only invites confusion. A more informative name wouldn't be out of line either - moNumber, or even moNr.

3. In methods that write output (such as DoWorkAmortization), it isn't a bad idea to pass in a PrintStream parameter rather than hard coding System.out.

Another 2 cents.