patternjavaMinor
Affordability/Mortgage Calculator
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
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
Your font specification is repeated throughout, you can set this as a
Variables naming
In these cases, you don't need temporary variables outside of the utility method to add them to the
Method and variable names should also be in
Miscellaneous
I think there is room for improvement for your code formatting inside the
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.