A command line Java program for computing grade point average

Question

(See the next iteration.)

I have this command line program for computing GPA (grade point average). Given credit sequence \$\langle c_1, c_2, \dots, c_n \rangle\$ and grade sequence \$\langle g_1, g_2, \dots, g_n \rangle\$ (I assume European grades, i.e., \$g_i \in \{ 1, 2, 3, 4, 5 \}\$ for all \$i \in \{1, 2, \dots, n \} \$), grade point average is

$$\frac{\sum_{i = 1}^n c_i g_i}{\sum_{i = 1}^n c_i}.$$

Being invoked without arguments, the program will expect input from keyboard. With one argument (which must be a CSV file with two columns), the program computes the GPA, prints it, and exits.

I tried to make the program robust: it reports all errors without crashing (except whenever the input file does not exist).

App.java

package net.coderodde.gpa;

import java.io.File;
import java.io.FileNotFoundException;
import java.util.ArrayList;
import java.util.List;
import java.util.Scanner;

/**
 * This class implements a command line application for computing GPA (grade 
 * point average).
 * 
 * @author Rodion "rodde" Efremov
 * @version 1.6 (Mar 20, 2016)
 */
public class App {

    private final List<Integer> creditList = new ArrayList<>();
    private final List<Integer> gradeList  = new ArrayList<>();
    private final Scanner scanner;

    public App(String fileName) {
        if (fileName == null) {
            scanner = new Scanner(System.in);
        } else {
            try {
                scanner = new Scanner(new File(fileName));
            } catch (FileNotFoundException ex) {
                throw new RuntimeException(
                        "File \"" + fileName + "\" does not exist.", ex);
            }
        }
    }

    public static void main(String[] args) {
        if (args.length > 1) {
            printHelp();
            return;
        }

        try {
            App app = new App(args.length == 0 ? null : args[0]);
            app.populateLists();
            System.out.println(app.computeGradePointAverage());
        } catch (Exception ex) {
            System.err.println("ERROR: " + ex.getMessage());
            System.exit(1);
        }
    }

    private void populateLists() {
        int lineNumber = 1;

        while (scanner.hasNextLine()) {
            String line = scanner.nextLine().trim();

            if (line.equals("end")) {
                return;
            }

            if (line.isEmpty()) {
                lineNumber++;
                continue;
            }

            if (!line.contains(",")) {
                System.err.println(
                        "Line " + lineNumber + " does not have a comma.");
                lineNumber++;
                continue;
            }

            String[] parts = line.split(",");

            if (parts.length != 2) {
                System.err.println(
                        "Line " + lineNumber + " contains invalid number of " +
                        "tokens (" + parts.length + "). Should have exactly " + 
                        "two: number of credits and the grade.");
            } else {
                String creditsString = parts[0].trim();
                String gradeString   = parts[1].trim();
                int credits;
                int grade;

                try {
                    credits = Integer.parseInt(creditsString);
                } catch (NumberFormatException ex) {
                    System.err.println(
                            "Invalid credit token on line " + lineNumber +
                            ": " + creditsString);
                    ++lineNumber;
                    continue;
                }

                try {
                    grade = Integer.parseInt(gradeString);
                } catch (NumberFormatException ex) {
                    System.err.println(
                            "Invalid grade token on line " + lineNumber +
                            ": "+ gradeString);
                    ++lineNumber;
                    continue;
                }

                creditList.add(credits);
                gradeList.add(grade);
            }   

            lineNumber++;
        }
    }

    private double computeGradePointAverage() {
        double creditSum = 0.0;
        double sum = 0.0;

        for (int i = 0; i < creditList.size(); ++i) {
            creditSum += creditList.get(i);
            sum += creditList.get(i) * gradeList.get(i);
        }

        return sum / creditSum;
    }

    private static void printHelp() {
        System.out.println("Usage: java -jar FILE.jar [grades.csv]");
        System.out.println("Omit the only argument in order to input " + 
                           "data from command line.");
    }
}

Please, tell me anything that comes to mind.

Answer 1

The D in SOLID stands for the Dependency Inversion Principle, which states that we should depend upon abstractions, not concretions. We can therefore improve the App class by depending on receiving an abstraction for reading in the data:

public App(InputStream in) {
        scanner = new Scanner(in);
}

And we would push determining which InputStream we receive up to the main method:

public static void main(String[] args) throws FileNotFoundException {
    if (args.length > 1) {
        printHelp();
        return;
    }

    InputStream in = args.length == 0 ? System.in : new FileInputStream(args[0]);
    App app = new App(in);
    app.populateLists();
    System.out.println(app.computeGradePointAverage());
}

In the original code, we are adding related data to two separate List<Integer>s. This means that we have to keep two pieces of data in sync across separate data structures. This may not seem like much of a burden here, but as applications grow more complicated and data structures are passed around, we can see data become harder to keep in sync. This also means that we are not capturing some semantic information about our data. If we pass creditList where we mean to pass gradeList, there is no way the compiler can tell us our mistake. To solve both of these problems, we can create a small class, which I will call Course:

public class Course {
    private final int credits;
    private final int grade;

    public Course(int credits, int grade) {
        this.credits = credits;
        this.grade = grade;
    }

    public int getCredits() { return credits; }
    public int getGrade() { return grade; }
}

With our new Course class, we can replace the two List<Integer>s with a single List<Course>. But wait, something interesting can now happen here. We've pulled the data we need for calculating our credit-weighted grade into here, so why don't we also pull the calculation as well?

public int getCreditWeightedGrade() {
    return getCredits() * getGrade();
}

We are starting to see a more coherent model emerge out of the problem. Looking now, we can see there are two distinct parts to the code: reading in the data, and doing calculations on the data. We can separate these two parts to make our code more modular.

We need a better name for the data we are working with, though. Since we are dealing with grade data at a university level, we can look to that domain for our name: Transcript. A transcript is a history of grade data for a student over their entire studies. This assumes that the grade data is for a single student, an assumption I am willing to make at this point. For a similar idea at pre-university levels, Report Card is an equivalent nomenclature.

So we have a part of the code that reads in a transcript, and part of the code that represents that transcript. Our List<Course> is the representative of the transcript itself. We will rename the App class to TranscriptReader to encapsulate that concept:

public class TranscriptReader {
    private Scanner in;

    public TranscriptReader(InputStream in) {
        scanner = new Scanner(in);
    }

    public List<Course> readTranscript() {
        List<Course> transcript = new ArrayList<Course>();
        int lineNumber = 1;

        while (scanner.hasNextLine()) {
            String line = scanner.nextLine().trim();

            if (line.equals("end")) {
                return;
            }

            if (line.isEmpty()) {
                lineNumber++;
                continue;
            }

            if (!line.contains(",")) {
                System.err.println(
                        "Line " + lineNumber + " does not have a comma.");
                lineNumber++;
                continue;
            }

            String[] parts = line.split(",");

            if (parts.length != 2) {
                System.err.println(
                        "Line " + lineNumber + " contains invalid number of " +
                        "tokens (" + parts.length + "). Should have exactly " + 
                        "two: number of credits and the grade.");
                continue;
            }

            String creditsString = parts[0].trim();
            String gradeString = parts[1].trim();
            int credits;
            int grade;

            try {
                credits = Integer.parseInt(creditsString);
            } catch (NumberFormatException ex) {
                System.err.println(
                        "Invalid credit token on line " + lineNumber +
                        ": " + creditsString);
                ++lineNumber;
                continue;
            }

            try {
                grade = Integer.parseInt(gradeString);
            } catch (NumberFormatException ex) {
                System.err.println(
                        "Invalid grade token on line " + lineNumber +
                        ": "+ gradeString);
                ++lineNumber;
                continue;
            }

            transcript.add(new Course(credits, grade));  

            lineNumber++;
        }

        return transcript;
    }
}

There are additional improvements to this class that @janos already discussed in his answer. We are pulling the main and computeGradePointAverage methods into a separate class at this point to have a cleaner separation:

public class Main {
    public static void main(String[] args) throws FileNotFoundException {
        if (args.length > 1) {
            printHelp();
            return;
        }

        InputStream in = args.length == 0 ? System.in : new FileInputStream(args[0]);
        TranscriptReader transcriptReader = new TrasnscriptReader(in);
        List<Course> transcript = transcriptReader.readTranscript();
        System.out.println(computeGradePointAverage(transcript));
    }

    private static double computeGradePointAverage(List<Course> transcript) {
        double creditSum = 0.0;
        double sum = 0.0;

        for(Course course : transcript) {
            creditSum += course.getCredits();
            sum += course.getCreditWeightedGrade();
        }

        return sum / creditSum;
    }
}

We can see how we've threaded our new classes through the system. We've made a few changes to the main method and a few more to the computeGradePointAverage method (like making it static and taking a List<Course>). But there still is this idea of a transcript floating around without truly representing it. There is also the computeGradePointAverage method sitting on our Main class when it would make much more sense on a transcript. Let's create that class:

public class Transcript implements Iterable<Course> {
    private final List<Course> courseList;

    public Transcript() {
        courseList = new ArrayList<Course>();
    }

    public void addCourse(Course course) {
        courseList.add(course);
    }

    @Override
    public Iterator<Course> iterator() {
        return courseList.iterator();
    }
}

We now have a class to hold the idea of a transcript. You'll note that it implements the Iterable<Course> interface. This was done so that we can still iterate over the courses to do our calculations (for now). We thread that through our code, so TranscriptReader#readTranscript becomes:

public Transcript readTranscript() {
    // No more List<Course>
    Transcript transcript = new Transcript();
    int lineNumber = 1;

    while(scanner.hasNextLine()) {
        // ...Read in our data...

        transcript.addCourse(new Course(credits, grade));
    }

    return transcript;
}

Our main method becomes:

    public static void main(String[] args) throws FileNotFoundException {
        if (args.length > 1) {
            printHelp();
            return;
        }

        InputStream in = args.length == 0 ? System.in : new FileInputStream(args[0]);
        TranscriptReader transcriptReader = new TrasnscriptReader(in);
        Transcript transcript = transcriptReader.readTranscript();
        System.out.println(computeGradePointAverage(transcript));
    }

And our calculateGradePointAverage method becomes:

    private static double computeGradePointAverage(Transcript transcript) {
        double creditSum = 0.0;
        double sum = 0.0;

        // This works the same because Transcript is iterable.
        for(Course course : transcript) {
            creditSum += course.getCredits();
            sum += course.getCreditWeightedGrade();
        }

        return sum / creditSum;
    }

Now, though, we see that calculateGradePointAverage is asking for a lot of things from Transcript. It makes more sense for computeGradePointAverage to be on Transcript. Let's move it there. We can also make some convenience methods on transcript to make the calculation read more simply:

public class Transcript implements Iterable<Course> {
    private final List<Course> courseList;

    public Transcript() {
        courseList = new ArrayList<Course>();
    }

    public void addCourse(Course course) {
        courseList.add(course);
    }

    @Override
    public Iterator<Course> iterator() {
        return courseList.iterator();
    }

    public double getGradePointAverage() {
        int totalCredits = getTotalCredits();
        int totalCreditWeightedGrades = 0;

        for(Course course : this) {
            totalCreditWeightedGrades += course.getCreditWeightedGrade();
        }

        return (1.0 * totalCreditWeightedGrades) / totalCredits;
    }

    // Convenience method to get all the credits.
    public int getTotalCredits() {
        int totalCredits = 0;

        for(Course course : this) {
            totalCredits += course.getCredits();
        }

        return totalCredits;
    }
}

In our Main class, computeGradePointAverage goes away, and the main method looks like:

public static void main(String[] args) throws FileNotFoundException {
    if (args.length > 1) {
        printHelp();
        return;
    }

    InputStream in = args.length == 0 ? System.in : new FileInputStream(args[0]);
    TranscriptReader transcriptReader = new TrasnscriptReader(in);
    Transcript transcript = transcriptReader.readTranscript();
    System.out.println(transcript.getGradePointAverage());
}

Answer 2

Exception handling

When the input file doesn't exist, you catch FileNotFoundException and rethrow a RuntimeException. But there's not much benefit in doing that. The outcome is little different from letting the original exception bubble up. So why not just let it bubble up, simplifying the code.

In this code:

    try {
        App app = new App(args.length == 0 ? null : args[0]);
        app.populateLists();
        System.out.println(app.computeGradePointAverage());
    } catch (Exception ex) {

The only line that may throw an exception is the constructor call. So only that should be in the try block.

And the only type of exception that we expect here is FileNotFoundException, so that should be in the catch statement instead of the not recommended generic Exception.

Unnecessary conditions

Instead of checking if a line contains a comma before splitting by a comma, you could just split and judge by the parts.length. The error message you already have for the case when parts.length seems good enough as it is, without adding a special message just about the comma.

Trimming while splitting

Instead of trimming the parts after the split, you could embed trimming logic in the splitting:

String[] parts = line.split("\\s*,\\s*");

Note that this is thanks to that line itself is already trimmed, otherwise there may be whitespace at the start of the first token and the end of the last.

Naming

App is a command line application for computing GPA. It could use a better name to reflect what it does.

populateLists is not very intuitive. What lists? How about readCreditsAndGrades.

Answer 3

I don't think you ever close the scanner, and you could perhaps tailor the try-and-catch in the main method to be more readable?Functionality wise, perhaps you could give the user the option of inserting the numbers at run time and then ending the input sequence with -1 or something of the sort? Or Maybe using some sort of form pop-up? You could even read those numbers into a .csv file and then read it into your program, to avoid overhauling the code.