HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavaMinor

Affordability/Mortgage Calculator

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
affordabilitycalculatormortgage

Problem

I'm working on a class project for my intro to Java class. My background is in accounting/finance so I decided to make this simple calculator that helps the user decide what they can afford and how much they can save on interest expense over the loan term. I'm just looking for general feedback on code structure and tips/advice on better ways to clean up the code or make it more efficient.

Regarding layout, I chose to challenge myself and create the GUI without using Window builder and just start typing out the code. As such, I used gridlayout for ease of organization.

```
import javax.swing.JFrame;

public class Main {

public static void main(String[] args) {
// TODO Auto-generated method stub
Calculator calculator = new Calculator();

calculator.setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);
calculator.pack();
calculator.setVisible(true);

}

}

import javax.swing.JFrame;
import java.awt.BorderLayout;
import java.awt.Color;
import java.awt.Font;
import java.awt.GridLayout;
import javax.swing.BorderFactory;
import javax.swing.JButton;
import javax.swing.JComponent;
import javax.swing.JPanel;
import javax.swing.JTextField;
import java.awt.event.ActionEvent;
import java.awt.event.ActionListener;
import java.text.NumberFormat;
import javax.swing.JLabel;
import javax.swing.JOptionPane;

public class Calculator extends JFrame {
private JButton b1, b2, b3, b4;
private JTextField t1, t2, t3, t4, t5, t6, t7;
private JLabel l1, l1a, l2, l3, l4, l5, l6, l7, l8, l9, l10;
private JLabel l11, l12, l13, l14, l15, l16, l17, l18, l19, l20;
private JLabel l21, l22, l23, l24, l25, l26;

public Calculator()
{
JPanel panel1 = new JPanel();
JPanel panel2 = new JPanel();
JPanel panel4 = new JPanel();

panel1.setLayout(new GridLayout(6,4,10,0));
panel1.setBorder(BorderFactory.createLineBorder(Color.GRAY));
panel2.setLayout(new GridLayout(2,1));
panel2.setBackground(new Color(83, 116, 246));
panel2.setBorder(BorderFactory.createLineBo

Solution

Avoiding code repetition

new Font("Arial Rounded MT Bold", Font.PLAIN, 12)


Your font specification is repeated throughout, you can set this as a static final variable so that your UI elements need to only reference one of it:

// inside Calculator
private static final Font DEFAULT_FONT = new Font("Arial Rounded MT Bold", Font.PLAIN, 12);

// usage
purchasePriceLabel = new JLabel("Purchase Price");
purchasePriceLabel.setFont(DEFAULT_FONT);


Variables naming

l1, t2, b3 are poor names for variables, as they give no context as to what they are. It is easier to read them as purchasePriceLabel, interestRateText and clearButton. Sometimes, if you do not need to refer to a UI element beyond the initial assignment, e.g. for those labels, you can even combine the usage of a utility method and method-call-inlining to simplify the declaration. For example:

private static JLabel createLabel(String text) {
    return createLabel(text, DEFAULT_FONT, DEFAULT_COLOR);
}

private static JLabel createLabel(String text, Font font, Color color) {
    JLabel label = new JLabel(text);
    label.setFont(font);
    label.setColor(color);
    return label;
}

// usage
panel.add(createLabel("Purchase Price"));
panel.add(createLabel("Some special text", LARGE_FONT, Color.RED));


In these cases, you don't need temporary variables outside of the utility method to add them to the JPanel.

Method and variable names should also be in camelCase, so that they are not confused with PascalCase class names. This means ButtonFunction.CalcPayInt() can be better renamed as ButtonFunction.getPaymentInterest(), and the PMI/DTI1/DTI2 class variables can be renamed as pmi/dti1/dti2 as well. Again, if you feel not using the short forms lend the variables more context, then you should use their long forms too.

Miscellaneous

I think there is room for improvement for your code formatting inside the CalcPayInt() method. For example, having indented lines makes it easier to understand that they follow the preceding line:

// original
monthlyPay1 = (loanAmount * ((monthlyIntRate * Math.pow((1 + monthlyIntRate),termMonths1))/
(Math.pow((1 + monthlyIntRate), termMonths1) - 1))) + monthlyTax;
totalInt1 = ((monthlyPay1 - monthlyTax) * termMonths1) - loanAmount;

// suggested improvement
monthlyPay1 = (loanAmount * ((monthlyIntRate * Math.pow((1 + monthlyIntRate),termMonths1))/
                    (Math.pow((1 + monthlyIntRate), termMonths1) - 1))) + monthlyTax;
totalInt1 = ((monthlyPay1 - monthlyTax) * termMonths1) - loanAmount;

Code Snippets

new Font("Arial Rounded MT Bold", Font.PLAIN, 12)
// inside Calculator
private static final Font DEFAULT_FONT = new Font("Arial Rounded MT Bold", Font.PLAIN, 12);

// usage
purchasePriceLabel = new JLabel("Purchase Price");
purchasePriceLabel.setFont(DEFAULT_FONT);
private static JLabel createLabel(String text) {
    return createLabel(text, DEFAULT_FONT, DEFAULT_COLOR);
}

private static JLabel createLabel(String text, Font font, Color color) {
    JLabel label = new JLabel(text);
    label.setFont(font);
    label.setColor(color);
    return label;
}

// usage
panel.add(createLabel("Purchase Price"));
panel.add(createLabel("Some special text", LARGE_FONT, Color.RED));
// original
monthlyPay1 = (loanAmount * ((monthlyIntRate * Math.pow((1 + monthlyIntRate),termMonths1))/
(Math.pow((1 + monthlyIntRate), termMonths1) - 1))) + monthlyTax;
totalInt1 = ((monthlyPay1 - monthlyTax) * termMonths1) - loanAmount;

// suggested improvement
monthlyPay1 = (loanAmount * ((monthlyIntRate * Math.pow((1 + monthlyIntRate),termMonths1))/
                    (Math.pow((1 + monthlyIntRate), termMonths1) - 1))) + monthlyTax;
totalInt1 = ((monthlyPay1 - monthlyTax) * termMonths1) - loanAmount;

Context

StackExchange Code Review Q#108917, answer score: 5

Revisions (0)

No revisions yet.