Savings account class and test program

Question

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:

1. ask the user the amount deposited into the account during that month. Then add the amount to the account balance.
2. ask the user for the amount withdrawn from the account during the month. The subtract the amount from the balance
3. 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 annualInterestRate; //Account annual interest rate
   private double monthlyInterestRate; 
   private double totalDeposits; 
   private double totalWithdraws; 
   private double totalInterest; 

   /**
     * Constructor
     * @param startBalance The account's balance. 
     * @param annual_Interest_Rate The annual interest rate. 
     */
    public SavingsAccount(double startBalance, double annual_Interest_Rate)
    {
        balance = startBalance;
        annualInterestRate = annual_Interest_Rate; 
    }
    //end of Constructor

   /**
    * setAnnualInterestRate method sets the annual interest 
    * rate and calculates the monthly interest rate
    * @param annual_Interest_Rate The annual interest rate.
    */
   public void setAnnualInterestRate(double annual_Interest_Rate)
   {
       monthlyInterestRate = annualInterestRate / 12; 
   }
   //end of setAnnualInterestRate method

   /**
    * The deposit method adds the amount to the balance  
    * and calculates the total deposit
    * @param amount
    */
   public void setDeposit(double amount)
   {
       balance += amount; 
       totalDeposits += amount; 
   }
   //end of deposit method

   /**
    * The withdraw method subtracts the amount to the balance
    * and calculates the total withdraws
    * @param amount
    */
   public void setWithdraw(double amount)
   {
       balance -= amount; 
       totalWithdraws += amount; 
   }
   //end of withdraw method

   /**
    * The calculateMonthlyInterest method calculates the total 
    * interest and adds the monthly interest to the balance 
    */
   public void calculateMonthlyInterest()
   {
       totalInterest = totalInterest + balance * monthlyInterestRate; 
       balance = balance + balance * monthlyInterestRate; 
   }
   //end of calculateMonthlyInterest method

   /**
    * The getBalance method returns the account's balance. 
    * @return The value of the balance field.
    */
   public double getBalance()
   {
       return balance; 
   }

   /**
    * The getAnnualInterestRate method returns the annual interest rate.
    * @return The value of the annual interest rate field.
    */
   public double getAnnualInterestRate()
   {
       return annualInterestRate; 
   }

   /**
    * The getMonthlyInterestRate method returns the monthly interest rate.
    * @return The value of the monthly interest rate field.
    */
   public double getMonthlyInterestRate()
   {
       return monthlyInterestRate; 
   }

   /**
    * The getTotalDeposits method returns the total deposit amount.
    * @return The value of the total deposits field.
    */
   public double getTotalDeposits()
   {
       return totalDeposits; 
   }

   /**
    * The getTotalWithdraws method returns the total withdraws amount.
    * @return The value of the total withdraws field.
    */
   public double getTotalWithdraws()
   {
       return totalWithdraws; 
   }

   /**
    * The getTotalInterest method returns the total interest amount.
    * @return The value of the total interest field.
    */
   public double getTotalnterest()
   {
       return totalInterest; 
   }

   /* 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);
   }
   //end of displayData method
}
//end of SavingsAccount class

Test class:

import java.util.Scanner; 

public class SavingsAccountTest
{
    public static void main(String[] args)
    {
        double startBalance; 
        double annual_Interest_Rate; 
        int months; 
        double deposit_Amount; 
        double withdraw_Amount; 

        //Create an object for Scanner class
        Scanner input = new Scanner(System.in); 

        //Prompt user for starting balance 
        System.out.print("Enter starting balance: $"); 
        startBalance = input.nextDouble(); 

        //Prompt user for annual interest rate
        System.out.print("Enter annual interest rate: "); 
        annual_Interest_Rate = input.nextDouble(); 

        //Prompt user for number of months
        System.out.print("Enter the number of months: ");
        months = input.nextInt(); 

        /* Create an object for SavingsAccount class */
        SavingsAccount sa = new
        SavingsAccount(startBalance, annual_Interest_Rate); 

        //Call to setAnnualInterestRate method
        sa.setAnnualInterestRate(annual_Interest_Rate);

        //Loop
        for (int i = 0; i < months; i++)
        {
            /* Prompt user for deposit amount */
            System.out.print("Enter amount to deposit for the month " + (i+1) + ":$"); 
            deposit_Amount = input.nextDouble(); 

            //Call to deposit method
            sa.setDeposit(deposit_Amount); 

            /* Prompt user for amount to withdraw */
            System.out.print("Enter amount to withdraw for the month " + (i+1) + ":$"); 
            withdraw_Amount = input.nextDouble(); 

            //Call to withdraw method
            sa.setWithdraw(withdraw_Amount); 

            /* Call to calculateMonthlyInterest method */
            sa.calculateMonthlyInterest();

        }
        //end of loop

        //Call to displayData method
        sa.displayData(); 
    }
    //end of main method
}
//end of SavingsAccountTest class

Answer 1

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 printing to the console. It also locks down the way the data can be used. It goes to the console, even if we'd rather have it go to a file, over the network, or into a GUI. Because it is locked down, the SavingsAccount class is less reusable. (Read up on the single responsibility principle.)

Since SavingsAccountTest is already concerned with console IO, this method would be better as a static method on that class.

---

Wrap Up

All the changes applied:

public class SavingsAccount
{
    private double balance;
    private double annualInterestRate;
    private double totalDeposits;
    private double totalWithdraws;
    private double totalInterest;

    /**
     * Constructor
     * @param startBalance The account's balance. 
     * @param annualInterestRate The annual interest rate. 
     */
    public SavingsAccount(double startBalance, double annualInterestRate)
    {
        balance = startBalance;
        this.annualInterestRate = annualInterestRate;
    }

    /**
     * Sets the annual interest rate
     * @param annualInterestRate The annual interest rate.
     */
    public void setAnnualInterestRate(double annualInterestRate)
    {
        this.annualInterestRate = annualInterestRate; 
    }

    /**
     * Adds the amount to the balance and calculates the total deposit
     * @param amount
     */
    public void deposit(double amount)
    {
        balance += amount; 
        totalDeposits += amount; 
    }

    /**
     * Subtracts the amount to the balance and calculates the total withdraws
     * @param amount
     */
    public void withdraw(double amount)
    {
        balance -= amount; 
        totalWithdraws += amount; 
    }

    /**
     * Calculates the total interest and adds the monthly interest to the balance 
     */
    public void accrueMonthlyInterest()
    {
        double monthlyInterest = balance * getMonthlyInterestRate();
        totalInterest += monthlyInterest; 
        balance += monthlyInterest; 
    }

    /**
     * @return The account's balance
     */
    public double getBalance()
    {
        return balance; 
    }

    /**
     * @return The annual interest rate
     */
    public double getAnnualInterestRate()
    {
        return annualInterestRate; 
    }

    /**
     * @return The monthly interest rate.
     */
    public double getMonthlyInterestRate()
    {
        return annualInterestRate / 12; 
    }

    /**
     * @return The total deposits
     */
    public double getTotalDeposits()
    {
        return totalDeposits; 
    }

    /**
     * @return The total withdraws
     */
    public double getTotalWithdraws()
    {
        return totalWithdraws; 
    }

    /**
     * @return The total interest
     */
    public double getTotalnterest()
    {
        return totalInterest; 
    }
}

Test class:

import java.util.Scanner; 

public class SavingsAccountTest
{
    public static void main(String[] args)
    {
        double startBalance; 
        double annualInterestRate; 
        int months; 
        double depositAmount; 
        double withdrawAmount; 

        //Using a Scanner so we can easily pull in different data types
        Scanner input = new Scanner(System.in); 

        //Prompt user for starting balance 
        System.out.print("Enter starting balance: $"); 
        startBalance = input.nextDouble(); 

        //Prompt user for annual interest rate
        System.out.print("Enter annual interest rate: "); 
        annualInterestRate = input.nextDouble(); 

        //Prompt user for number of months
        System.out.print("Enter the number of months: ");
        months = input.nextInt(); 

        SavingsAccount sa = new SavingsAccount(startBalance, annualInterestRate); 

        for (int i = 1; i <= months; i++)
        {
            // Prompt user for deposit amount
            System.out.print("Enter amount to deposit for the month " + i + ":$"); 
            depositAmount = input.nextDouble(); 

            sa.setDeposit(depositAmount); 

            // Prompt user for amount to withdraw
            System.out.print("Enter amount to withdraw for the month " + i + ":$"); 
            withdrawAmount = input.nextDouble(); 

            sa.setWithdraw(withdrawAmount); 

            sa.accrueMonthlyInterest();

        }

        displayData(sa); 
    }

    /**
     * Displays the details of the savings account
     */
    public static void displayData(SavingsAccount sa)
    {
        double balance = Math.round(sa.getBalance() * 100.0) / 100.0; 
        double totalInterest = Math.round(sa.getTotalInterest() * 100.0) / 100.0; 
        System.out.println(); 
        System.out.println("The ending balance is: $" + balance); 
        System.out.println("Total amount of deposits: $" + sa.getTotalDeposits());
        System.out.println("Total amount of withdraws: $" + sa.getTotalWithdraws());
        System.out.println("Total interest earned: $" + totalInterest);
    }
}

Answer 2

Comments

Comments should be there to explain something that the code itself can't. Many of your comments just repeat information already expressed just as well by the code you're commenting. For example:

//Call to setAnnualInterestRate method
sa.setAnnualInterestRate(annual_Interest_Rate);

The comment isn't adding any information here. The line below is clearly a call to that method, there's no need to say that twice.

Comments like this are actually a form of repetition, so it arguably violates the DRY (Don't Repeat Yourself) principle. To see this, imagine you had to change that line to call something else instead. Now you have two places to update rather than one- the line itself and its comment.

I don't think the "end of..." comments are all that useful either. These usually only serve any purpose when you have lots of nesting and large blocks, and in that case they're more of a code smell that you're doing something wrong.

In cases where the code doesn't express enough, maybe it's the code that should change rather than adding a comment. For example:

private double balance; //Account Balance

Is the comment because it's not clear what "balance" alone means? Then change the variable name to accountBalance and lose the comment.

State consistency

The problem description requires being able to do things with both the monthly and annual interest rate. However, that does NOT mean you necessarily need a field for both of them.

As it stands, SavingsAccount now has a requirement for being in a valid state: It much be the case that monthlyInterestRate = annualInterestRate\12.

It's not inherently a problem that your class has a requirement like this. They are referred to as invariants, and as long as you don't publicly expose anything that allows any calling code to break it, it's fine for a class to protect its own invariant.

In this specific case, though, it's not just an invariant but also a DRY violation- you're representing the same knowledge in two different places. It would be easier to just store a single version of the interest rate, and have a private method to translate it into the other version when needed. This reduces the potential for bugs, since you aren't always having to update two values when you really only want to change one thing.

Note that you do already have bugs of this form: the constructor only sets the annual interest rate, and setAnnualInterestRate only sets the monthly rate.

Naming

Your naming is generally good, but you switch between camelCase and snake_Case arbitrarily. You should drop the underscores.

Also don't automatically add "set" when it's not needed to a name. setDeposit is a strange phrase, and would be more natural as addDeposit or makeDeposit.

I'd also consider renaming calculateMonthlyInterest. calculate implies it's going to give me back the answer to some question, but actually it's changing the underlying state. Something like addInterestForMonth or even advanceMonth might be more expressive.

Separation of Concerns

You generally do a really good job of separating out concerns, the only place this falls down is in the displayData method. This should return a string rather than printing to screen. That way your SavingsAccount doesn't care about what kind of IO you're using, and you could just as easily use the same class save that information in a file, send it through a webservice, email it to someone, show it in a GUI, etc.

Misc

• calculateMonthlyInterest could make use of +=.
• System.out.print("Enter the number of months: "); - This isn't very clear given the question description. Why not something a little friendlier like "How many months old is the account?"
• Your getters and setters are required by the problem statement. Hypothetically though, if that statement wasn't there, I'd remove most of them until I had a reason to need them, since they reduce encapsulation.

Answer 3

When creating a class you should think about implementing the following constructors and which ones you will need. They help the clarity, functionality, and also predictability of your code.

• Default Constructor: This is created by the JVM if you don't define it and there is not a non-default constructor available. However, if you are using a default constructor alone, it's best to create this yourself and not leave it up to the JVM. This makes sure that everything is instantiated in the way that you intended. If you have a strict requirement to not allow an object to be created with default values, don't create this, and create the parameter-accepting constructor instead.

---

     public savingsAccount () {
           this.totalInterest = 0; //Instead of 0, use your default values
           this.balance = 0;
           //Do this process for all variables in class, establishing defaults that you create.
     }

---

• Copy Constructor: You are going to want to make copies of your objects at one point, or another. This constructor makes it simple and expands the flexibility of your classes. Even if you don't end up using it often in the program, the added flexibility is worth the small amount of memory in my opinion.

---

     public savingsAccount (savingsAccount s2) {
           this.totalInterest = s2.totalInterest;
           this.balance = s2.balance;
           //Do this process for all variables in class
     }

---

• Parameter-accepting Constructor: This one you have already created in your code. You need a way to instantiate an object with specific values and this constructor allows you to do that. By creating this, the JVM will no longer create a default constructor. By choosing this as the only constructor in your class, you create the case that no object of the class can be created without passing values to it upon instantiation.
  

---

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