Grade report builder

Question

I am starting out with Java programming and get limited feedback from my professor. Following is an assignment that I handed in, and just re-factored. I wanted to see if people with Java experience could give some pointers on improving how it looks. Last semester I took a C++ course so splitting sections up into public/private methods/classes/objects is pretty foreign to me. Feedback on commenting is also welcome; I'm not sure about how much I should be commenting.

Note: All user input is taken is as a string as per the Professor's request. Hence the parseDoubles flying around.

    import java.util.Scanner;

public class ReportBuilder {

    static Scanner keyboard = new Scanner(System.in);

    public static int numericalInputs = 7;

    private static String firstName;
    private static String lastName;
    static String reportName;

    public static void main(String[] args) {

        firstName = "Empty";
        lastName = "Empty";
        reportName = "Empty";

        System.out.println("This program will generate a report based on the input");
        System.out.println("If you wish to exit enter \"-1\"");

        System.out.println("Please enter your first name:");
        firstName = FirstInput.main();

        System.out.println(firstName + ", please enter your last name one character at a time");
        System.out.println("When you are finished, type 'Stop'");
        lastName = SecondInput.main();

        System.out.println("Hi, " + firstName + " " + lastName + ".");
        System.out.println("Enter the name of the report:");
        reportName = keyboard.next();

        if (reportName.equals("-1")) {
            exit();
        }
        else {
        System.out.println("Please enter seven numbers one at a time");
        ThirdInputv2.main();
        }

        keyboard.close();   
    }

    public static void exit() {
        System.out.println("Good bye!");
        System.exit(0);
    }
}

public class FirstInput {

    public static String main() {

        String ownerName = "Empty";

        ownerName = ReportBuilder.keyboard.next();
        if (ownerName.equals("-1")) {
            ReportBuilder.exit();
        }
        else {
            ownerName = ownerName.replaceAll("[0-9]+ ?","");
        }

        return ownerName;
    }
}

public class SecondInput {

    private static String lastName;
    private static int stringLength;

    public static String main() {
        lastName = "Empty";

        return characterInput();
    }

    private static String characterInput() {
        /* This method takes in the last name one character at a time then stops when instructed.
         */

        String[] charArray = new String[20];
        String lastInput = "None";

        stringLength = -1;

        for (int i = 0; i < charArray.length; i++) {    
            /* This loop takes in individual strings and creates an array of strings of length 1.
             */
            lastInput = ReportBuilder.keyboard.next();
            charArray[i] = lastInput.substring(0,1);
            if (lastInput.equals("-1")) {
                ReportBuilder.exit();
            }

            /* The validateLast method of CheckNum is called to see if the user input contains a number.
             */
            else if (CheckNum.validateLast(charArray[i])) {
                System.out.println("Please do not enter any numbers");
                i--;
            }

            /* When the user enters "Stop" no more input is accepted
             */
            else if (lastInput.equalsIgnoreCase("Stop")) {
                stringLength = i;
                break;
            }
        }

        lastName = stringCharacters(charArray, stringLength);

        return lastName;
    }

    private static String stringCharacters(String[] characters, int length) {
        /* This method concatenates all characters 
         * then turns them into a single string, containing the last name.
         */
        String lastName = "";

        for (int i = 0; i < length; i++) {
            lastName = lastName + characters[i];
        }
        return lastName;
    }
}

public class ThirdInputv2 {

    private static int numberOfInputs = ReportBuilder.numericalInputs;
    private static String[] userInputString = new String[numberOfInputs];

//  The following arrays are created so that each one can be fed into the grandTotal method
    private static double[] arrayOfInputs = new double[numberOfInputs];
    private static double[] arrayOfMaxValues = new double[numberOfInputs];
    private static double[] arrayOfMinValues = new double[numberOfInputs];
    private static double[] arrayOfAccumulators = new double[numberOfInputs];
    private static double[] arrayOfAverages = new double[numberOfInputs];

    public static void main() {
        numArray();
        System.out.println("");
        printReport();
    }

    private static void numArray() {
        /* This method collects each number entered by the user then stores it in an array
         * After each number is entered: the max value, minimum value, average, and total are displayed
         */ 

        int elementCounter = 1;

        for(int i = 0; i < numberOfInputs; i++) {           
            userInputString[i] = ReportBuilder.keyboard.next();

            // Program exit condition of -1:
            if (userInputString[i].equals("-1")) {
                ReportBuilder.exit();
            }

            if (i == 0); {
                arrayOfAccumulators[i] = Double.parseDouble(userInputString[i]);
                arrayOfAverages[i] = arrayOfAccumulators[i];

            }

            arrayOfInputs[i] = Double.parseDouble(userInputString[i]);

            if (i > 0) {
                arrayOfAccumulators[i] = arrayOfAccumulators[i-1] + arrayOfInputs[i];
                arrayOfAverages[i] = arrayOfAccumulators[i-1]/elementCounter;
            }

            arrayOfMaxValues[i] = maxCalc(arrayOfInputs, elementCounter);
            arrayOfMinValues[i] = minCalc(arrayOfInputs, elementCounter);

            elementCounter++;

            printMetrics(arrayOfInputs[i], arrayOfMaxValues[i], arrayOfMinValues[i], 
                         arrayOfAccumulators[i], arrayOfAverages[i], elementCounter);
            }
        }

    private static double maxCalc(double[] arrayNums, int counter) {
        /* This method finds the greatest element in the incoming array.
         */

        double highest = -1;
        highest = arrayNums[0];

        for(int i = 0; i<counter; i++) {
            if(arrayNums[i] > highest) {
                highest = arrayNums[i];
            }       
        }
        return highest;
    }

    private static double minCalc(double[] arrayNums, int counter) {
        /* This method finds the lowest element in the incoming array.
         */

        double lowest = -1;
        lowest = arrayNums[0];

        for (int i = 0; i<counter; i++) {
            if(arrayNums[i] < lowest) {
                lowest = arrayNums[i];
            }
        }
        return lowest;
    }

    private static void printMetrics(double input, double max, double min, double accumulator, double average, int counter) {
        /* This method prints the metrics for the latest user input value.
         */
        System.out.println("The running total is " + accumulator);
        System.out.println("The max value so far is " + max
                            + " and the min value is " + min);
        System.out.println("The average value is " + average);

        if(counter < numberOfInputs) {
            System.out.println("Please enter the next number:");
        }
    }

    private static void printReport() {
        /* This method prints out the final report to screen.
         */

        String name = ReportBuilder.reportName;

        // Header
        System.out.println("Report ID: " + name);
        System.out.println("Input" + "\t" + "Max" + "\t" + "Min" + "\t" + "Total" + "\t" + "Average");
        System.out.println("----------------------------------------");

        // Body
        for (int row = 0; row < numberOfInputs; row++) {
            // This loop prints out the contents of each column, row by row

            System.out.println(arrayOfInputs[row] + "\t" + arrayOfMaxValues[row] + "\t" + arrayOfMinValues[row]
                               + "\t" + arrayOfAccumulators[row] + "\t" + arrayOfAverages[row]);
        }

        // Grand Total row containing the sum of each column
        System.out.println("----------------------------------------");
        System.out.println(grandTotal(arrayOfInputs) + "\t" + grandTotal(arrayOfMaxValues) + "\t"
                           + grandTotal(arrayOfMinValues) + "\t" + grandTotal(arrayOfAccumulators) + "\t"
                           + grandTotal(arrayOfAverages) + "  = Grand Totals");
    }

    private static double grandTotal(double[] columnArray) {
        /* This method calculates the sum of all values for a given column in the report.
         */

        double gTotal = columnArray[0];
        for (int i = 1; i < 7; i++) {
            gTotal = gTotal+ columnArray[i];
        }

        return gTotal;
    }
}

public class CheckNum {

    public static boolean validateLast(String testString) {
        /* The variable isNumeric returns true if the input string contains an integer between 0 and 9
         */
        boolean isNumeric;

        if(testString.matches("[0-9]+")) {
            isNumeric = true;
        }
        else {
            isNumeric = false;
        }

        return isNumeric;
    }
}

Answer 1

Just a few comments:

• in Java, comments describing methods are ususally placed right before the method declaration, not after it. Also, use /** ... */, so they are turned into Javadoc comments.
• Its odd how you create separate classes for each of the input tasks, and how those classes define a main method. In Java, the main method describes the entry point to your program, like the main method in your first class. Calling other methods main may lead to confusion.
• Rather than having several classes with lots of static variables, try to convert those classes to simple methods of your main class, and make those variables local to those methods.

---

Actually I did not plan on doing this, but whatever. Here's my version of the code.

import java.util.Scanner;

/**
 * [describe what the program is doing]
 */
public class ReportBuilder {

    static Scanner keyboard = null;

    /**
     * main program flow
     */
    public static void main(String[] args) {
        keyboard = new Scanner(System.in);

        System.out.println("This program will generate a report based on the input");
        System.out.println("If you wish to exit enter \"-1\"");

        System.out.println("Please enter your first name:");
        String firstName = getFirstName();

        System.out.println(firstName + ", please enter your last name one character at a time");
        System.out.println("When you are finished, type 'Stop'");
        String lastName = getLastName();

        System.out.println("Hi, " + firstName + " " + lastName + ".");
        System.out.println("Enter the name of the report:");
        String reportName = readString();

        System.out.println("Please enter seven numbers one at a time");
        double[] numbers = getNumberArray(7);
        System.out.println("");
        printReport(reportName, numbers);

        keyboard.close();   
    }

    /**
     * handle exit condition each time a keyboard input is read
     */
    private static String readString() {
        String input = keyboard.next();
        if ("-1".equals(input)) {
            System.out.println("Good bye!");
            System.exit(0);
        }
        return input;
    }

    private static String getFirstName() {
        String firstName = readString();
        return firstName.replaceAll("[0-9]+ ?","");
    }

    /** This method takes in the last name one character at a time then stops when instructed.
     */
    private static String getLastName() {
        String lastName = "";
        while (true) {
            String input = readString();
            // When the user enters "Stop" no more input is accepted
            if (input.equalsIgnoreCase("Stop")) {
                break;
            }
            // The validateLast method of CheckNum is called to see if the user input contains a number.
            if (isNumber(input)) {
                System.out.println("Please do not enter any numbers");
            }
            // add character to lastName
            lastName += input.substring(0, 1);
        }
        return lastName;
    }

    /** This method collects each number entered by the user then stores it in an array
     * After each number is entered: the max value, minimum value, average, and total are displayed
     */ 
    private static double[] getNumberArray(int numberOfInputs) {
        double[] inputs = new double[numberOfInputs];
        double min = 9999, max = 0, sum = 0, avg = 0;

        for (int i = 0; i < numberOfInputs; i++) {           
            String userInput = readString();

            if (isNumber(userInput)) {
                inputs[i] = Double.parseDouble(userInput);
                min = Math.min(min, inputs[i]);
                max = Math.max(max, inputs[i]);
                sum = sum + inputs[i];
                avg = sum / (i+1);

                System.out.println("The running total is " + sum);
                System.out.println("The max value so far is " + max
                        + " and the min value is " + min);
                System.out.println("The average value is " + avg);

                if (i < numberOfInputs) {
                    System.out.println("Please enter the next number:");
                }
            } else {
                System.out.println("Please enter a number!");
                i--;
            }
        }
        return inputs;
    }

    /** This method prints out the final report to screen.
     */
    private static void printReport(String name, double[] numbers) {
        double min = 9999, max = 0, sum = 0, avg = 0;
        double[] mins = new double[numbers.length];
        double[] maxs = new double[numbers.length];
        double[] sums = new double[numbers.length];
        double[] avgs = new double[numbers.length];

        // Header
        System.out.println("Report ID: " + name);
        System.out.println("Input" + "\t" + "Max" + "\t" + "Min" + "\t" + "Total" + "\t" + "Average");
        System.out.println("----------------------------------------");

        // Body
        // This loop prints out the contents of each column, row by row
        for (int i = 0; i < numbers.length; i++) {
            // okay, here's a bit of code duplication, but not that critical
            mins[i] = min = Math.min(min, numbers[i]);
            mins[i] = max = Math.max(max, numbers[i]);
            sums[i] = sum = sum + numbers[i];
            avgs[i] = avg = sum / (i+1);

            System.out.println(numbers[i] + "\t" + max + "\t" + min + "\t" + sum + "\t" + avg);
        }

        // Grand Total row containing the sum of each column
        System.out.println("----------------------------------------");
        System.out.println(sum(numbers) + "\t" + sum(maxs) + "\t" + sum(mins) 
                + "\t" + sum(sums) + "\t" + sum(avgs) + "  = Grand Totals");
    }

    /** This method calculates the sum of all values for a given column in the report.
     */
    private static double sum(double[] nums) {
        double sum = 0;
        for (int i = 1; i < nums.length; i++) {
            sum += nums[i];
        }
        return sum;
    }

    /** @return true if the input string contains an integer
     */
    private static boolean isNumber(String testString) {
        return testString.matches("[0-9]+");
    }
}