5
\$\begingroup\$

My job for now is to teach a trainee how to program. I told him to solve the following task, given by a book (I try to translate it):

Write a program for compound interest calculation. The user has to enter the amount of money, the years, and the interest rate. The program should look like this:

Money in Euro: 100
Annual percentage rate: 6
Term in years: 4
Money after 1 years: 106.0
Money after 2 years: 112.36
Money after 3 years: 119.1016
Money after 4 years: 126.247696

Important Note

He hasn't learned anything about OOP, exception handling or using objects and methods yet. He now learns how to design simple algorithms. But he has gotten a hint how to use the scanner class to get the user input.

I want to show him my own example solution, so that he can learn. I want to ask you, if my example solution is okay for showing it to a beginner. Is it well written or have I ignored any good practices?

// use "int variable = scanner.nextInt()" for getting int values
// use "double variable = scanner.nextDouble()" for getting double values

import java.util.Scanner;

public class Main {
    private static Scanner scanner = new Scanner(System.in);

    public static void main(String args[]) {
        // write your solution here:    
        double money;
        int years;
        double rate;

        System.out.print("Money in Euro: ");
        money = scanner.nextDouble();
        System.out.print("Annual percentage rate: ");
        rate = scanner.nextDouble();
        System.out.print("Term in years: ");
        years = scanner.nextInt();

        for (int i = 1; i <= years; i++) {
            money += money * (rate / 100);
            System.out.println("Money after " + i + ". years: " + money);
        }
    }
}
\$\endgroup\$
  • \$\begingroup\$ I mean since I started learning programming just like that guy (trying to read inputs without knowing what OOP is) I can ensure you that if he is into programming and as long as you tell him what the methods do (and he does not need to think about how they work) he will get it... \$\endgroup\$ – Cedced_Bro Jan 16 at 16:05
1
\$\begingroup\$

A few subjective points...

Isolation

Going forward, you're going to want to isolate the UI from the algorithm as much as possible. So, using methods to break up the algorithm as suggested by @Doi9t is definitely worth considering. I think it's also worth considering making the output stream a class field for the moment. This is going to be closer to the log approach that can be more prevalent in larger programs, so it's good to get into the habit early.

Something like:

private static final PrintStream CONSOLE = System.out;

and used:

CONSOLE.print("Money in Euro: ");

Redundancy

Generally you want to minimise the amount of work that's done within a loop (it has to be done every time). So if you can do a bit of work beforehand then it can make the loop more efficient as well as making it clearer what's happening. With that in mind, a small change to calculate the yearly increase rate would change your calculation to:

double yearlyIncreaseRate = 1.0 + (rate/100);

for (int i = 1; i <= years; i++) {
    money *= yearlyIncreaseRate;
    CONSOLE.println("Money after " + i + ". years: " + money);
}

final

I like to use final whenever I can, to indicate that I'm not expecting a variable to change. As it stands, you're not expecting most of your inputs to change. So I'd consider declaring them as final, which ties in nicely with declaring the variables at the same point as their first usage:

final double rate = scanner.nextDouble();

naming

Your variable names are quite concise. For a small program that's probably fine, however the cost of having more descriptive names is small and it can make algorithms easier to follow. annualInterestRate vs rate, termYears vs years.

\$\endgroup\$
3
\$\begingroup\$

I have some suggestion for you.

1) Constant name should always be uppercase.

private static final Scanner SCANNER = new Scanner(System.in);

2) When creating arrays, I suggest that you use the java style instead of the c style, since it's the more used and less error-prone, in my opinion.

Before

public static void main(String args[]) {
   //[...]
}

After

public static void main(String[] args) {
   //[...]
}

3) For the variables, I suggest that you create them on the same line as the initialization, you will save 3 lines in the method and make the code more readable.

        System.out.print("Money in Euro: ");
        double money = SCANNER.nextDouble();
        System.out.print("Annual percentage rate: ");
        double rate = SCANNER.nextDouble();
        System.out.print("Term in years: ");
        int years = SCANNER.nextInt();

4) You can extract the questions in methods, to separate the logic and it will be easier to read, if you want; but will have to explain to him :)

public static void main(String args[]) {
    // write your solution here:

    double money = askForAmountInEuro();
    double rate = askForAnnualPercRate();
    int years = askForTerm();

    for (int i = 1; i <= years; i++) {
        money += money * (rate / 100);
        System.out.println("Money after " + i + ". years: " + money);
    }
}

private static int askForTerm() {
    System.out.print("Term in years: ");
    return SCANNER.nextInt();
}

private static double askForAnnualPercRate() {
    System.out.print("Annual percentage rate: ");
    return SCANNER.nextDouble();
}

private static double askForAmountInEuro() {
    System.out.print("Money in Euro: ");
    return SCANNER.nextDouble();
}
```
\$\endgroup\$
  • 1
    \$\begingroup\$ I personally like it when all variable declarations appear at the top of a method. Delphi for example forces the programmer to do that, because of readability. But okay, that is a matter of opinion. \$\endgroup\$ – Dexter Thorn Jan 17 at 10:19
1
\$\begingroup\$

A potential issue is related to the static Scanner object:

private static Scanner scanner = new Scanner(System.in);

This scenario can bring potential problems due to the closing of the Scanner resource left to the developer. From Java 8 it is possible to use the try-with-resources statement to ensure closing of resources (in this case the Scanner object to avoid memory leaks). A possible solution is rewrite of the code including declaration and definition of the Scanner inside the main like below:

public class Main {

    public static void main(String args[]) {
        try (Scanner scanner = new Scanner(System.in)) {
            System.out.print("Money in Euro: ");
            double money = scanner.nextDouble();
            System.out.print("Annual percentage rate: ");
            double rate = scanner.nextDouble();
            System.out.print("Term in years: ");
            int years = scanner.nextInt();

            for (int i = 1; i <= years; i++) {
                money += money * (rate / 100);
                System.out.println("Money after " + i + ". years: " + money);
            }
        }
    }
}

In this way the Scanner resource will be closed after its use in the main method.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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