Unbreakable average program

Question

I'm creating a program that averages four numbers - three from a file and one from user input. Simple enough. But the catch is, the professor is going to be deliberately trying to break the program.

I'd like advice in any regard, but remember the focus is on robustness.

import java.util.*;
import java.io.*;

public class average {
    public static void main (String[] args) throws IOException{
        double[] nums = new double[4];
        nums = inputHandler();
        System.out.println("Average " + findAverage(nums));

        System.out.println("End Program.");
    }

    public static double findAverage(double[] nums){
        double average = 0.0;
        final double N = 4.0;

        for(int i = 0; i < N; i++){
            average += nums[i] / N;
        }

        return average;
    }

    public static double[] inputHandler() throws IOException{
        Scanner input = new Scanner(System.in);
        double[] nums = new double[4];
        double[] fileNums = new double[3];

        // get first three numbers from file
        System.out.print("Enter the name of the file which contains the first three numbers: ");
        fileNums = fileInput(input.nextLine());
        System.out.println();

        // copy numbers from fileNums[] to nums[]
        for(int i = 0; i < 3; i++){
            nums[i] = fileNums[i];
        }

        //get last number from user input
        System.out.print("Enter the fourth number: ");
        nums[3] = userInput(input.nextLine());
        System.out.println();

        input.close();

        return nums;
    }

    public static double[] fileInput(String fileName) throws IOException{
        double[] nums = new double[3];

        System.out.println();

        File file = new File(fileName);

        if(!file.isFile()){
            System.err.println("ERROR: File does not exist.");
            System.exit(-1);
        }

        System.out.println(fileName + ":");

        Scanner input = new Scanner(file);
        String cur = "";

        for(int i = 0; i < 3; i++){

            if(!input.hasNext()){
                System.err.println("ERROR: File does not contain enough numbers.");
                System.exit(-1);
            }

            cur = input.next();

            System.out.println("Number " + (i + 1) + ": " + cur);

            if(cur.matches("-?\\d+(\\.\\d+)?")){ //regex ensures numeric input
                if(Double.parseDouble(cur) < 1.7e308){
                    nums[i] = Double.parseDouble(cur);
                }
                else{
                    System.err.println("ERROR: Number too large");
                    System.exit(-1);
                }
            }
            else{
                System.err.println("ERROR: Non numeric input. Please check your file and try again.");
                System.exit(-1);
            }
        }

        input.close();

        return nums;
    }

    public static double userInput(String userInput){
        double num = 0;

        if(userInput.matches("-?\\d+(\\.\\d+)?")){ //regex ensures input is numeric
            num = Double.parseDouble(userInput);
        }
        else{
            System.err.println("ERROR: Non numeric input. Please check your file and try again.");
            System.exit(-1);
        }

        return num;
    }
}

Answer 1

First, use try/catch blocks around parts that can throw exceptions. For example, what happens when an IOException is thrown by the file, perhaps because the data is corrupted and the file exists, but cannot be opened?

Second, I don't know what constitutes failure, but an app-closing failure is failure in my book:

if(!file.isFile()){
    System.err.println("ERROR: File does not exist.");
    System.exit(-1);
}

Third, @SirPython is right - use the built-in method for reading your number. There are built-in functions provided for reading both doubles and ints, whichever your program needs.

Fourth, this will not necessarily help robustness, split your method logic up. You are doing a lot of printing in your fileInput() method. Do your printing in a dedicated method. Do your input in a dedicated method. Feel free to throw an exception when exceptional behavior happens, but be sure to handle the exception. Split your logic up into bite-sized components and implement them separately - this will make reuse easy.

Fifth, recover from your errors. If you need, start the entire input processes over, but do not close the program. Closing the program because of an error will probably be counted as an error, and is unacceptable in many programs.

Answer 2

The program cannot be made totally unbreakable.

for(int i = 0; i < N; i++){
    average += nums[i] / N;
}

for(int i = 0; i < N; i++){
    total += nums[i];
}
average = total / N;

There are two ways to compute the average of a list of numbers, both of which can be broken:

1. The method you use: divide each number by the size of the list, then add it to the running total. This is immune to overflow errors, but is more vulnerable to rounding errors than method #2, particularly with exceptionally small numbers (if the professor enters four copies of Double.MIN_VALUE, the code will return 0 when it should return Double.MIN_VALUE).
2. Add up all the numbers and divide the total by the size of the list. This is resistant to rounding errors, but is vulnerable to overflow errors (if the professor enters four copies of Double.MAX_VALUE, it will overflow when it should return Double.MAX_VALUE). It is also faster than method #1, but in most situations, the speed difference doesn't matter.

The correct one to pick depends on whether overflow errors or rounding errors are more important.

Answer 3

public static double userInput(String userInput){
    // ...
    if(userInput.matches("-?\\d+(\\.\\d+)?")){
        // ...
    } else {
        System.err.println("ERROR: Non numeric input.Please check your file and try again.");
        System.exit(-1);
    }
    // ...
}

I was prompted to enter a number! I may have entered it wrongly, but why does it ask me to check my file?

This is the problem with copying-and-pasting code... If you have a method that accepts a double from a Scanner instance and a custom error message, you will be able to side-step this problem. For example:

/**
 * Gets the number of double values from the scanner.
 *
 * @param scanner      the {@link Scanner} instance to take from
 * @param times        the number of values required
 * @param errorMessage the error message to use if not enough values
 * @return a double array of the required size
 * @throws {@link IllegalArgumentException} if not enough values
 */
private static double[] getInputs(Scanner scanner, int times, String errorMessage) {
    double[] results = new double[times];
    int i = 0;
    for (; i < times && scanner.hasNextDouble(); i++) {
        results[i] = scanner.nextDouble();
    }
    if (i != times) {
        throw new IllegalArgumentException(errorMessage);
    }
    return results;
}

Also, to echo @Hosch250's answer, robustness of a program is its ability to recover from errors... what you have done here is only to handle them with error messages, but you didn't attempt to recover. This may be something worth looking into too, such as are you expected to keep prompting for the fourth number until a valid double value is accepted?

Answer 4

Useless code

On the first line of code, I already saw a problem:

    double[] nums = new double[4];
    nums = inputHandler();

Why do you allocate an array for nums and then immediately reassign it to something else? The first line is completely rendered useless. It should be:

    double[] nums = inputHandler();

You made the same mistake in inputHandler() with fileNums.

Average function

There are a couple of things I don't like about this function:

public static double findAverage(double[] nums){
    double average = 0.0;
    final double N = 4.0;

    for(int i = 0; i < N; i++){
        average += nums[i] / N;
    }

    return average;
}

1. You use a double, N, as your loop limit.
2. You divide once per loop instead of just once.

Here's how I would write it. Note that my function assumes the array will contain at least one element. If that isn't necessarily true, it could be made to check for null or empty arrays also.

public static double findAverage(double[] nums){
    double sum = 0.0;
    final int numElements = nums.length;

    for(int i = 0; i < numElements; i++) {
        sum += nums[i];
    }

    return sum / numElements;
}

Answer 5

You are doing way too much work to validate the input; the Scanner already has a method for reading doubles: java.util.Scanner.nextDouble.

Rather than doing all this crazy regex work, you should just use this along side java.util.Scanner.hasNextDouble to receive input.

Using this will ensure that all input received is valid and that it is received efficiently. Therefore, it will be very difficult for your professor to break the application.

---

In Java, it is bad practice to just say, for example, that a method is going to throw an IOException without actually doing any handling. In fact, if you are using an IDE, your IDE will most likely throw a temper tantrum about it.

All these methods that throw IOException should be manually throwing it, not relying on the method calling this to handle it.

Just insert a try/catch in the method and have the method handle the exception, rather than whatever uses that method.

---

final double N = 4.0;

If this is a constant value, then it is better practice to move this to a property of the class and declare it like this:

public static final double N = 4.0;

---

You have this number 3 showing up in a lot of places in your code. It looks kind of awkward just sitting there; it should be in a constant, similar to above.

Answer 6

If the user pipes a stream of characters that does not contain a newline to stdin, the Scanner will probably just keep reading. (I tested with a 9 mb file and it read the whole thing.)

Scanner.next() will do the same thing when you try to read the next line from the data file.

It may also be possible to get the scanner to hang when reading through file system trickery. For example if the file is replaced between the call to File.isFile() and the attempt to read it, it could be replaced by a pipe that just hangs.