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

Savings account class and test program

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

Problem

My code is complete. It runs properly and produces the correct output. I did calculations by hand to check and then ran the program and it gives me the same result. I just want a second opinion. Are there ways for my code to be more efficient? Are there small details that I need to change? My professor marked me off for tiny errors, so I want to cover all the bases. I included the instructions down below just in case.


Design a SavingsAccount class that stores a savings account's balance, annual interest rate. The class constructor should accept the amount of savings account's starting balance and annual interest rate. The class should also have methods for subtracting the amount of a withdrawal, adding the amount of a deposit, and adding the amount of monthly interest to the balance. The monthly interest rate is the annual interest rate divided by 12. To add the monthly interest to the balance, multiply the monthly interest rate by the balance and add the amount to the balance. The class should also has mutator and accessor methods for each data field.


Then write a test program that calculate the balance of a savings account at the end of a period of time. The test program should ask the user the annual interest rate, the starting balance, and the number of months that have passed since the account was established. Then a loop should iterate once for every month, performing the following:



  • ask the user the amount deposited into the account during that month. Then add the amount to the account balance.



  • ask the user for the amount withdrawn from the account during the month. The subtract the amount from the balance



  • calculate the monthly interest





After the last iteration, the program should display the ending balance, the total amount of deposits, the total amount of withdrawals, and the total interest earned.

```
public class SavingsAccount
{
//Data fields
private double balance; //Account Balance
private double annualI

Solution

First, the convention in Java is camelCase, not camel_Snake_Case. Variables like annual_Interest_Rate should be annualInterestRate. If you are worried because this creates a name collision between a parameter and a class property, the class property can be prefixed with this. for specificity, so:

public SavingsAccount(double startBalance, double annualInterestRate)
{
    balance = startBalance;
    this.annualInterestRate = annualInterestRate; 
}


public void setAnnualInterestRate(double annual_Interest_Rate)
{
    monthlyInterestRate = annualInterestRate / 12;
}


The first big flag here is that there is a parameter that is not being used in this method. The second big flag is that it doesn't do what it says it does: it never actually sets annualInterestRate. Looking deeper, we can see other issues with monthlyInterestRate. I don't think you should be storing monthly interest rate at all in your class. It is easy to calculate on the fly, and harder to make sure it is synced with annualInterestRate. Instead, you should do:

public double getMonthlyInterestRate()
{
    return annualInterestRate / 12; 
}


then, in your code where you use monthlyInterestRate, replace it with getMonthlyInterestRate():

public void calculateMonthlyInterest()
{
    totalInterest = totalInterest + balance * getMonthlyInterestRate();
    balance = balance + balance * getMonthlyInterestRate();
}


Next, the calculateMonthlyInterest method. Internally it does a calculation, but it does not return the results of that calculation. This makes the name a little misleading. A better name might be accrueMonthlyInterest. Inside of that method, you have lines:

totalInterest = totalInterest + balance * monthlyInterestRate;
balance = balance + balance * monthlyInterestRate;


You already use += and -= elsewhere, and they can be used even when the calculation is more that just a single number or variable. So we can shorten the above lines to:

totalInterest += balance * monthlyInterestRate;
balance += balance * monthlyInterestRate;


We can now see we have the same number calculated twice in a row. This is a good candidate for extracting to a temporary variable so the computer doesn't have to do the math twice, and so we make sure that we use the same number both times:

double monthlyInterest = balance * monthlyInterestRate;
totalInterest += monthlyInterest;
balance += monthlyInterest;


Methods like setDeposit and setWithdraw are misleading. They add or deduct, not set. Instead deposit and withdraw would be better names.

Now on to comments. Ideally, comments shouldn't state the obvious, echo the implementation, be wrong, or be imprecise.

//Data fields
private double balance; //Account Balance
private double annualInterestRate; //Account annual interest rate


All of these comments state the obvious, and are unnecessary.

//end of Constructor


All comments like this state the obvious, and are unnecessary. Your methods here are short, and easy to find the end of. If you are making very long methods, and find yourself needing bookmarks like this, instead try to break a large method up into smaller, more focused methods.

/**
 * setAnnualInterestRate method sets the annual interest 
 * rate and calculates the monthly interest rate
 * @param annual_Interest_Rate The annual interest rate.
 */


This comment, as noted earlier, is wrong, but we're going to fix that. It also echos the implementation that monthly interest is stored internally. The method name and word "method" in all the comments are redundant as well.

//Create an object for Scanner class


States the obvious, echos implementation. Better might be something like: // Using a Scanner so we can easily pull in different data types. That explains why a Scanner is being used.

/* Create an object for SavingsAccount class */


Once again, states the obvious. Additionally, // should be for single-line comments, while / / should be for multi-line comments.

for (int i = 0; i < months; i++)
{


Fine loop, but everywhere you have i, it's as (i+1). There's no requirement that a loop start at 0. We can do:

for (int i = 1; i <= months; i++)
{


then we can just use i.

/* displayData method displays the ending details of the savings account */
public void displayData()
{
    balance = Math.round(balance * 100.0) / 100.0; 
    totalInterest = Math.round(totalInterest * 100.0) / 100.0; 
    System.out.println(); 
    System.out.println("The ending balance is: $" + balance); 
    System.out.println("Total amount of deposits: $" + totalDeposits);
    System.out.println("Total amount of withdraws: $" + totalWithdraws);
    System.out.println("Total interest earned: $" + totalInterest);
}


I would say this does not belong in the SavingsAccount class. A class mostly concerned with tracking account information suddenly is also concerned with

Code Snippets

public SavingsAccount(double startBalance, double annualInterestRate)
{
    balance = startBalance;
    this.annualInterestRate = annualInterestRate; 
}
public void setAnnualInterestRate(double annual_Interest_Rate)
{
    monthlyInterestRate = annualInterestRate / 12;
}
public double getMonthlyInterestRate()
{
    return annualInterestRate / 12; 
}
public void calculateMonthlyInterest()
{
    totalInterest = totalInterest + balance * getMonthlyInterestRate();
    balance = balance + balance * getMonthlyInterestRate();
}
totalInterest = totalInterest + balance * monthlyInterestRate;
balance = balance + balance * monthlyInterestRate;

Context

StackExchange Code Review Q#84551, answer score: 7

Revisions (0)

No revisions yet.