3
\$\begingroup\$

New to Java, just finished the Helsinki MOOC course. I programmed a basic mortgage calculator as my first solo project and while it works, I have a few questions about coding style:

  1. The listener and ui classes are cluttered with variables at the top. Is this acceptable or is there a cleaner way of organizing it?

  2. Is initializing the variables in the CalcLogic class as strings and then creating new BigDecimal variables in the methods an accepted way of using BigDecimal, or would it better to initialize the variables as BigDecimals?

  3. Are there any other coding style or logic issues that are noticeable?

CalcLogic class:

package mortgagecalculator.logic;

import java.math.BigDecimal;
import java.math.RoundingMode;

/**
 * Logic for mortgage calculator.
 * Input variables for home value, loan term, etc. and outputs EMI, total payment, etc.
 * 
 */

public class CalcLogic {

    //Variables for calculating mortgage. propTax, homeIns, etc are not initialized in constructor
    private String principal = "0";
    private String downPayment = "0";
    private String interestRate = "0";
    private int loanTerm = 0;
    private String propTax = "0";
    private String homeIns = "0";
    private String hoaFee = "0";
    private String pmiRate = "0";

    public void setPrincipal(String principal) {
        this.principal = principal;
    }

    public void setDownPayment(String downPayment) {
        this.downPayment = downPayment;
    }

    public void setInterest(String interestRate) {
        this.interestRate = interestRate;
    }

    public void setLoanTerm(int loanTerm) {
        this.loanTerm = loanTerm;
    }    

    public void setPropTax(String propTax) {
        this.propTax = propTax;
    }

    public void setHomeIns(String homeIns) {
        this.homeIns = homeIns;
    }

    public void setHOAfee(String hoaFee) {
        this.hoaFee = hoaFee;
    }

    public void setPMIrate(String pmiRate) {
        this.pmiRate = pmiRate;
    }

    //Returns principal minus downpayment 
    public BigDecimal getInitialAmount() {
        return new BigDecimal(this.principal).subtract(new BigDecimal(this.downPayment));
    }

    //Returns twenty percent of principal
    public BigDecimal twentyDown() {
        BigDecimal twenty = new BigDecimal("20").divide(new BigDecimal("100"));
        return new BigDecimal(this.principal).multiply(twenty);
    }

    public BigDecimal monthlyInterest() {
        return new BigDecimal(this.interestRate).divide(new BigDecimal("100"))
                             .divide(new BigDecimal("12"), 8, RoundingMode.UP);
    }

    public BigDecimal monthlyPMI() {

        try {
           BigDecimal result = getInitialAmount().divide(new BigDecimal(this.pmiRate).divide(new BigDecimal("100"))
                                 .divide(new BigDecimal("12")), 2, RoundingMode.UP);
           return result;

        } catch (Exception e) {
           return BigDecimal.ZERO;
        }

    }

    public BigDecimal additionalTax() {
        BigDecimal sum = new BigDecimal(this.propTax);
        sum = sum.add(new BigDecimal(this.homeIns)).add(new BigDecimal(this.hoaFee));      
        return sum;
    }    

    public BigDecimal getEMI() {
        BigDecimal value = monthlyInterest().add(BigDecimal.ONE);
        value = value.pow(this.loanTerm * 12);

        BigDecimal numerator = value.multiply(monthlyInterest()).multiply(getInitialAmount());
        BigDecimal denominator = value.subtract(BigDecimal.ONE);

        return numerator.divide(denominator, 2, RoundingMode.UP);

    }

    public BigDecimal monthlyPayment() {
        BigDecimal result = getEMI().add(monthlyPMI()).add(additionalTax());
        return result;
    }

    public BigDecimal totalInterest() {
        BigDecimal result = getEMI().multiply(new BigDecimal(this.loanTerm * 12));
        result = result.subtract(getInitialAmount());
        return result;
    }

    //Returns month where principal payments first exceed interest payments
    public int amortMonth() {
        BigDecimal remaining = getInitialAmount().setScale(2, RoundingMode.CEILING);

        for (int i = 0; i < this.loanTerm * 12; i++) {

            BigDecimal interPay = remaining.multiply(monthlyInterest()).setScale(2, RoundingMode.CEILING);            
            BigDecimal princPay = getEMI().subtract(interPay).setScale(2, RoundingMode.CEILING);

            if (princPay.compareTo(interPay) == 1) {
                return i + 1;
            }

            remaining = remaining.subtract(princPay).setScale(2, RoundingMode.CEILING);

        }

        return 0;
    } 

}

CalcListener class:

package mortgagecalculator.ui;

import mortgagecalculator.logic.CalcLogic;
import java.awt.event.ActionEvent;
import java.awt.event.ActionListener;
import javax.swing.JTextField;
import javax.swing.JButton;
import java.math.BigDecimal;

public class CalcListener implements ActionListener {

    private JTextField principal;
    private JTextField downPayment;
    private JTextField interestRate;
    private JTextField loanTerm;
    private JTextField propTax;
    private JTextField homeIns;
    private JTextField hoaFee;
    private JTextField pmiRate;
    private JButton button;
    private JTextField EMI;
    private JTextField monthlyFee;
    private JTextField totalInt;
    private JTextField changeMonth;
    private CalcLogic calc;

    public CalcListener(JTextField principal, JTextField downPayment, JTextField interestRate,
                        JTextField loanTerm, JTextField propTax, JTextField homeIns,
                        JTextField hoaFee, JTextField pmiRate, JTextField EMI,
                        JTextField monthlyFee, JTextField totalInt, JTextField changeMonth, JButton button) {
        this.principal = principal;
        this.downPayment = downPayment;
        this.interestRate = interestRate;
        this.loanTerm = loanTerm;
        this.propTax = propTax;
        this.homeIns = homeIns;
        this.hoaFee = hoaFee;
        this.pmiRate = pmiRate;
        this.button = button;
        this.EMI = EMI;
        this.monthlyFee = monthlyFee;
        this.totalInt = totalInt;
        this.changeMonth = changeMonth;
        this.calc = new CalcLogic();
    }

    @Override
    public void actionPerformed(ActionEvent ae) {

        if (ae.getSource() == this.button) {

            this.calc.setPrincipal(this.principal.getText());
            this.calc.setDownPayment(this.downPayment.getText());
            this.calc.setInterest(this.interestRate.getText());
            this.calc.setLoanTerm(Integer.parseInt(this.loanTerm.getText()));
            this.calc.setPropTax(this.propTax.getText());
            this.calc.setHomeIns(this.homeIns.getText());
            this.calc.setPMIrate(this.pmiRate.getText());
            this.calc.setHOAfee(this.hoaFee.getText());

            this.EMI.setText(this.calc.getEMI().toString());
            this.monthlyFee.setText(this.calc.monthlyPayment().toString());
            this.totalInt.setText(this.calc.totalInterest().toString());
            this.changeMonth.setText(String.valueOf(this.calc.amortMonth()));            

        }

    }
}

UserInterface class:

package mortgagecalculator.ui;

import java.awt.Container;
import java.awt.Dimension;
import java.awt.BorderLayout;
import java.awt.GridLayout;
import javax.swing.*;
import mortgagecalculator.ui.CalcListener;

public class UserInterface implements Runnable {

    private JFrame frame;
    private JTextField principal;
    private JTextField downPayment;
    private JTextField interestRate;
    private JTextField loanTerm;
    private JTextField propTax;
    private JTextField homeIns;
    private JTextField hoaFee;
    private JTextField pmiRate;
    private JButton button;
    private JTextField EMI;
    private JTextField monthlyFee;
    private JTextField totalInt;
    private JTextField changeMonth;

    @Override
    public void run() {
        frame = new JFrame("Mortgage Calculator");
        frame.setPreferredSize(new Dimension(1000, 1000));
        frame.setDefaultCloseOperation(WindowConstants.EXIT_ON_CLOSE);

        createComponents(frame.getContentPane());

        frame.pack();
        frame.setVisible(true);
    }


    private void createComponents(Container container) {
        generateVariables();
        container.setLayout(new GridLayout(3, 1));

        container.add(createInputPanel());
        container.add(this.button);
        container.add(createOutputPanel());

    }

    private void generateVariables() {
        this.principal = new JTextField("");
        this.downPayment = new JTextField("");
        this.interestRate = new JTextField("");
        this.loanTerm = new JTextField("");
        this.propTax = new JTextField("");
        this.homeIns = new JTextField("");
        this.hoaFee = new JTextField("");
        this.pmiRate = new JTextField("");
        this.EMI = new JTextField("");
        this.monthlyFee = new JTextField("");
        this.totalInt = new JTextField("");
        this.changeMonth = new JTextField("");
        this.button = new JButton("Calculate!");

        CalcListener calc = new CalcListener(this.principal, this.downPayment, this.interestRate,
                                             this.loanTerm, this.propTax, this.homeIns, this.hoaFee,
                                             this.pmiRate, this.EMI, this.monthlyFee, this.totalInt,
                                             this.changeMonth, this.button);

        this.button.addActionListener(calc);
    }

    private JPanel createInputPanel() {
        JPanel panel = new JPanel(new GridLayout(4, 4));
        panel.add(new JLabel("Principal"));
        panel.add(new JLabel("Down Payment"));
        panel.add(new JLabel("Interest Rate (in %)"));
        panel.add(new JLabel("Loan term (in years)"));
        panel.add(this.principal);
        panel.add(this.downPayment);
        panel.add(this.interestRate);
        panel.add(this.loanTerm);

        panel.add(new JLabel("Property Tax (Monthly)"));
        panel.add(new JLabel("Home Insurance (Monthly)"));
        panel.add(new JLabel("HOA Fees (Monthly)"));
        panel.add(new JLabel("PMI Rate (in %)"));
        panel.add(this.propTax);
        panel.add(this.homeIns);
        panel.add(this.hoaFee);
        panel.add(this.pmiRate);

        return panel;
    }

    private JPanel createOutputPanel() {
        JPanel panel = new JPanel(new GridLayout(4, 2));

        panel.add(new JLabel("Equated Monthly Installment"));
        panel.add(new JLabel("Monthly w/ fees"));
        panel.add(this.EMI);
        panel.add(this.monthlyFee);

        panel.add(new JLabel("Total Interest Payments"));
        panel.add(new JLabel("Month where principal payments > interest"));
        panel.add(this.totalInt);
        panel.add(this.changeMonth);

        return panel;
    }

    public JFrame getFrame() {
        return frame;
    }
}
\$\endgroup\$

1 Answer 1

Reset to default
2
\$\begingroup\$

CalcLogic should only deal with BigDecimal values; not String values.

Consider the symmetry of input and output:

class CalcLogic {
    public void       setPrinciple(String principle) { ... }
    public BigDecimal getInitialAmount() { ... }
}

or

class CalcLogic {
    public void       setPrinciple(BigDecimal principle) { ... }
    public BigDecimal getInitialAmount() { ... }
}

Why would you setXxxx() a String value and getXxxx() a BigDecimal? This class deals with numbers, specifically money. Money values go in; money values come out.

Along that vein, you should write the member functions following the Java Bean standard. Instead of monthlyInterest(), you should name the method getMonthlyInterest().

The CalcListener class is breaking encapsulation. It requires exactly the same fields as a the UserInterface class, and all those fields have to be passed in the constructor. If you add a field, you have to add code in many places to ensure the class definitions are kept in sync.

Instead, CalcListener could be an inner class.

public class UserInterface implements Runnable {

    private JTextField principal;
    // ... etc ...

    class CalcListener implements ActionListener {

        private final CalcLogic calc = new CalcLogic();

        @Override
        public void actionPerformed(ActionEvent ae) {
            calc.setPrinciple(new BigDecimal(principle.getText()));
            // ... etc ...
        }
    }

    private void generateVariables() {
        // ... etc ...
        button.addActionListener(new CalcListener());
    }

    // ... etc ...
}

As an inner class, CalcListener has access to all the private members of UserInterface, so there is no need to pass them all in a call to the CalcListener() constructor.

It is still breaking encapsulation, albeit in a different way - one which requires a lot less code.

Instead of using JTextField, you probably want to use JFormattedTextField.

The JFormattedTextField can perform the work of converting between the display format, a String, and the model object, in this case a BigDecimal.

Use a helper function to create the JFormattedTextField, to set it up properly.

private static JFormattedTextField bigDecimalField(String initialValue, String format) {
    BigDecimal value = new BigDecimal(initialValue);
    JFormattedTextField tf = new JFormattedTextField(value);
    DefaultFormatter fmt = new NumberFormatter(new DecimalFormat(format));
    fmt.setValueClass(value.getClass());
    DefaultFormatterFactory factory = new DefaultFormatterFactory(fmt, fmt, fmt);
    tf.setFormatterFactory(factory);

    return tf;
}

Create the fields like:

private final JFormattedTextField principle = bigDecimalField("0.00", "#.00");
// ...etc..

And reference the values like:

        @Override
        public void actionPerformed(ActionEvent ae) {
            calc.setPrinciple((BigDecimal) principle.getValue());
            // ... etc ...
        }

Notice, you are no longer retrieving a text string from the field, but the actual model object ... a BigDecimal.

Of course, you can do better by declaring your own BigDecimalField class, which returns the correctly cast return value, so the caller doesn't have to cast all the time.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.