Graduation planner

Question

I know this is not very good. I am currently in my first programming class (Java). It seems like it works, but I am sure there are things I have not thought of.

The goal is to have a user enter

• the number of credit units each class they need to take is worth, one by one
• the number of credit units to complete in a term

The program should calculate

• the number of remaining credit units
• the number of terms it will take to complete the degree
• the cost

import java.util.ArrayList;
import java.util.Scanner;

public class GraduationPlanner {

    public static void main(String[] args) {
        int sum = 0;
        String creditUnits;
        int unitsLeft;
        int plannedUnits = 0;
        int termsLeft = 0;
        double tuitionCost = 0;
        final double TUITION_RATE = 2890;
        ArrayList<Integer> cu = new ArrayList<Integer>();
        Scanner in = new Scanner(System.in);
        System.out.println("Please input the Credit Units for each course. Press enter after each course. Press Q when finished:  ");
        while (in.hasNextInt()) {
            int temp = (in.nextInt());
            if (temp < 0) {
                System.out.print("ERROR! Please enter a number 1-9.: ");
            } else {
                cu.add(temp);
            }
        }
        System.out.println(cu);
        Scanner ex = new Scanner(System.in);
        System.out.print("Please enter the number of Credit Units you will complete per term: ");
        if (ex.hasNextInt()) {
            plannedUnits = ex.nextInt();
        }
        for (int a : cu) 
        {
            sum += a;
        }
        unitsLeft = (sum - plannedUnits);

        termsLeft = (sum / plannedUnits);
        tuitionCost = (termsLeft * TUITION_RATE);
        System.out.println("After this term there are " + unitsLeft + " credit units remaining.");
        System.out.println("You will complete your degree in " + termsLeft + " terms.");
        System.out.print("It will cost you " + tuitionCost + " to complete your degree.");

    }
}

New code block

/* Determines the length of time and cost of a degree program
 * based on user inputs.
 */
import java.util.ArrayList;
import java.util.Scanner;

public class GraduationPlanner {

    public static void main(String[] args) {
        final double TUITION_RATE = 2890;
        ArrayList<Integer> creditUnits = new ArrayList<Integer>();
        Scanner unitsPerCourse = new Scanner(System.in);

        GetCreditUnits(unitsPerCourse, creditUnits);

        System.out.println(creditUnits); 
        System.out.print("Please enter the number of Credit Units you will complete per term: ");

        Scanner goalUnits = new Scanner(System.in);

        int plannedUnits = GetPlannedUnits(goalUnits);

        int creditUnitsSum = TotalOfCreditUnits(creditUnits);

        double tuitionCost;
        int termsLeft = CalculateTermsToComple(creditUnitsSum, plannedUnits);

        int unitsLeft;
        unitsLeft = CalculateUnitsLeft(creditUnitsSum, plannedUnits);

        tuitionCost = CalculateTuition(termsLeft, TUITION_RATE);

        System.out.println("After this term there are " + unitsLeft + " credit units remaining.");
        System.out.println("You will complete your degree in " + termsLeft + " terms.");
        System.out.print("It will cost you " + tuitionCost + " to complete your degree.");
        unitsPerCourse.close();
        goalUnits.close();
    }

    public static void GetCreditUnits(Scanner unitsPerCourse, ArrayList<Integer> creditUnits) {
        System.out.println("Please input the Credit Units for each course. Press enter after each course. Press Q when finished:  ");
        while (unitsPerCourse.hasNextInt()) {
            int currentUnit = (unitsPerCourse.nextInt());
            if (currentUnit < 1 || currentUnit >= 10) {
                System.out.print("ERROR! Please enter a number 1-9.: ");
            } else {
                creditUnits.add(currentUnit);
            }

        }
    }

    public static int GetPlannedUnits(Scanner goalUnits) {
        int plannedUnits = 0;
        if (goalUnits.hasNextInt()) {
            plannedUnits = goalUnits.nextInt();
        } else {
            System.out.print("ERROR! Please enter an integer!: ");
        }
        return plannedUnits;
    }

    public static int CalculateTermsToComple(int creditUnitsSum, int plannedUnits) {
        int termsLeft = (int) Math.ceil((double) creditUnitsSum / plannedUnits);
        return termsLeft;
    }

    public static int CalculateUnitsLeft(int creditUnitsSum, int plannedUnits) {
        int unitsLeft;
        unitsLeft = creditUnitsSum - plannedUnits;
        return unitsLeft;
    }

    public static double CalculateTuition(int termsLeft, final double TUITION_RATE) {
        double tuitionCost;
        tuitionCost = termsLeft * TUITION_RATE;
        return tuitionCost;
    }

    public static int TotalOfCreditUnits(ArrayList<Integer> creditUnits) {
        int creditUnitsSum = 0;
        for (int unit : creditUnits) {
            creditUnitsSum += unit;
        }
        return creditUnitsSum;
    }
}

Answer 1

Your program is a good starting point. It's not perfect but it doesn't have to be (I've seen code much worse than yours written by people studying programming for longer than you have).

Now, our goals when writing programs are:

1. Make it work
2. Make it readable.

If your program works, then congratulations -- the first step is complete. Here goes the actual review.

Many of the variables you used have decent names (plannedUnits, etc.), but we can do better:

1. The variables cu and a: What are those? cu seems to be the list that stored course units, so I'm guessing that's what cu stands for. But characters are cheap, so let's rename cu to courseUnits and a to courseUnit!
2. sum is reasonable -- it tells us that it is keeping track of the sum of something. But what exactly? It seems to be course units, so let's call it courseUnitSum or sumOfCourseUnits.
3. You use two variables (in and ex). Rename them to courseUnitsScanner and plannedUnitsScanner.
4. temp is not very clear either. Maybe newCourseUnit?
5. creditUnits is not used anywhere, is it? If you don't need it, then delete it.

In second place, I see you declaring tons of variables early but only using them late in the code. Try to declare variables as late as possible -- and let's see what we can come up with

import java.util.ArrayList;
import java.util.Scanner;

public class GraduationPlanner {

    public static void main(String[] args) {
        System.out.println("Please input the Credit Units for each course. Press enter after each course. Press Q when finished:  ");
        Scanner in = new Scanner(System.in);
        ArrayList<Integer> courseUnits = new ArrayList<Integer>();
        while (in.hasNextInt()) {
            int newCourseUnit = (in.nextInt());
            if (newCourseUnit < 0) {
                System.out.print("ERROR! Please enter a number 1-9.: ");
            } else {
                courseUnits.add(newCourseUnit);
            }
        }
        System.out.println(courseUnits);

        System.out.print("Please enter the number of Credit Units you will complete per term: ");
        Scanner ex = new Scanner(System.in);
        int plannedUnits = 0;
        if (ex.hasNextInt()) {
            plannedUnits = ex.nextInt();
        }

        int sumOfCourseUnits = 0;
        for (int courseUnit : courseUnits) 
        {
            sum += courseUnit;
        }
        int unitsLeft = (sum - plannedUnits);
        System.out.println("After this term there are " + unitsLeft + " credit units remaining.");

        int termsLeft = (sumOfCourseUnits / plannedUnits);
        System.out.println("You will complete your degree in " + termsLeft + " terms.");
        final double TUITION_RATE = 2890;
        double tuitionCost = (termsLeft * TUITION_RATE);
        System.out.print("It will cost you " + tuitionCost + " to complete your degree.");

    }
}

I see you writing int variableName = (something + someOtherThing);. Those parenthesis are not needed. Just write int variableName = something + someOtherThing;. Though of course having too many parenthesis doesn't hurt.

System.out.print("ERROR! Please enter a number 1-9.: ");

Heh. You say "1-9" but you only check if it is negative. If 0 and 10 are not acceptable values, then use:

if (temp < 1 || temp > 9) {

sum / plannedUnits. This will round the result down. Is that what you want? For instance, if there are 20 units and you plan 6 units, you'll have 3 terms (not 4). My suggestion would be:

int termsLeft = (int)Math.ceil((double)sum / plannedUnits);

but I use "explicit type casts" here, and I suspect you might have not learned about those yet. So an alternative is:

int termsLeft = sum / plannedUnits;
if (sum % plannedUnits != 0) {
   termsLeft++;
}

Aside from this, you have no comments and you are using just a single method -- but I don't know if you've learned what any of those words mean, so I'd rather not overwhelm you. It's okay for your work not to be perfect the first time.

One warning, though:

    if (ex.hasNextInt()) {
        plannedUnits = ex.nextInt();
    }

What happens if the user does not input an integer (and instead writes regular text)? Will plannedUnits remain zero? If so, beware -- you'll be dividing by zero later on!

---

UPDATE: You updated your code so it's only fair that I update my review.

Your new version is much better than the first one.

That said, you have missed one important piece of @Lstor's advice: Your "Get" functions have I/O.

About unitsPerCourse: This is a weird name for a Scanner. I'd expect a number or maybe a Map for a variable with this name. This is not a variable that contains the unitsPerCourse, it's the variable that handles asking the user about unitsPerCourse.

Also, in Java, it is common for method names to start with a lowercase letter, so GetPlannedUnits would be getPlannedUnits.

The use of uppercase TUITION_RATE is very common for constants, but weird for function parameter names.

final in parameters means the function is not allowed to change the value of the parameter -- but the code that uses the function can pass any value it wants. You never change the value of parameters anyway, so it doesn't make much sense to make TUITION_RATE one. I'd do one of the following:

1. Change it to a normal parameter (tuitionRate in CalculateTuition, but keep it called TUITION_RATE in main);
2. Remove the parameter and move TUITION_RATE to the CalculateTuition.
3. Move CalculateTuition to the class level.

This code could be simplifed:

public static int CalculateUnitsLeft(int creditUnitsSum, int plannedUnits) {
    int unitsLeft;
    unitsLeft = creditUnitsSum - plannedUnits;
    return unitsLeft;
}

Why separate the declaration of unitsLeft and its assignment? You can just do:

int unitsLeft = creditUnitsSum - plannedUnits;

In fact, we can do even better:

public static int CalculateUnitsLeft(int creditUnitsSum, int plannedUnits) {
    return creditUnitsSum - plannedUnits;
}

Finally, have you learned about "exceptions" yet? If not, then this won't make much sense to you, but if/when you do, the usual way to handle resources that must be closed is:

Scanner unitsPerCourse = new Scanner(System.in);
try {
   //Your code here
} finally {
    unitsPerClose.close();
}

Newer versions of Java (Java 7) have a shorter version that does the same (called try-with-resources):

try (Scanner unitsPerCourse = new Scanner(System.in) {
    //Your code here
}

I hope this helps.

Answer 2

I have been teaching Java to new students for some years, and I have seen a lot of beginner Java code. For a complete beginner, your code isn't horrible -- and that's high praise! For example, your variable naming is fairly decent, especially for a beginner. (But cu isn't a good name. Change that.) I can't see anything in your code that immediately strikes me as Don't do that!!!, and that is rare for beginner code :-)

There are of course a lot of things to improve upon. I'm only going to state some of the parts I consider most urgent, because you need to learn one step at a time. Once you master the basic principles, you can move on.

The small steps

First of all, you should divide your code into logical segments. Preferably, this should be various methods/functions, but start by doing logical divisions inside your main function. Group the code lines into logical units, and separate those units with a single empty line. You don't want to create your variables earlier than needed. If you keep them close to where they are used, i.e. make them part of the code they logically belong to, it's easier to remember what they are for, and to avoid using them in the wrong place. For example, move int sum = 0; down to right above

for (int a : cu) {
        sum += a;
}

Note that I changed your for loop to adhere to the Java conventions, like the rest of your code does.

Functions

After that, you should split your code into functions. Try to make a function perform a single logical computation. Knowing what "a single computation" really means is hard, but you will get it with experience. The old Unix mantra is nice: "Do one thing, and do it well."

Do not fall for the temptation to move variables into class scope when you introduce functions. Pass them as parameters instead.

The reason you want to use functions is to reduce complexity. Which of these two snippets is easier to read?

int sum = calculateSum(numbers);

or...

int sum = 0;
for (int i : numbers) {
    sum += i;
}

Exactly. Neither of the snippets are rocket science, but the former is much easier to read. Functions are also used to make the code easier to reuse and Bob Loblaw, but don't think too much about that yet.

Note that since you want a function to do a single item of work, you don't want functions that do both computations and input/output. In your case, you should probably do I/O in main and call functions to do the real calculation.

When you have incorporated these changes and are happy with your program, you could post it as a new question for further guidelines.

Answer 3

A few things I noted:

You never close your scanners. That's a resource leak, which isn't good. Always close your Scanners when you are done with them:

in.close();

---

Your logic condition here:

if (temp < 0)
{
    System.out.print("ERROR! Please enter a number 1-9.: ");
}

Right now if you enter 0, it accepts it. In the error message, it says to only enter a number 1-9:

if (temp < 1 || temp > 9)
{
    // ...
}