Simple factorial program using recursion

Question

Two concepts I realized I needed to understand and use more are recursion and Exceptions. Thus, I combined both in the following program. Although it began with a focus on using recursion it became more about Exceptions. I think the recursive part is bare-bones and straight-forward but I would nonetheless appreciate any tips/advice on using/formatting recursion in general.

On exception-handling: Is the following the optimal way of employing them or is there a more conventionally favored fashion? I get the feeling I may be missing doing something unnecessary or missing something important.

import javax.swing.JOptionPane;

class FactorialDemo {
    public static void main(String[] args) {
        int factorialNumber = 0;
        try {
            factorialNumber = Integer.parseInt(JOptionPane.showInputDialog(null,
                "Which number should we compute the factorial of?"));
        } catch(NumberFormatException nfe) {
            JOptionPane.showMessageDialog(null,"Input must be an integer!",
                "Error", JOptionPane.ERROR_MESSAGE);

            throw new NumberFormatException("Input must be an integer.");
        }

        JOptionPane.showMessageDialog(null,
                factorialNumber + " factorial is " +recur(factorialNumber) +".",
                "Result", JOptionPane.PLAIN_MESSAGE);
    }

    public static int recur(int n) {
        int result;
        if (n < 0) {
            JOptionPane.showMessageDialog(null,"n must be >= 0 but was " + n,
                "Error", JOptionPane.ERROR_MESSAGE);

            throw new IllegalArgumentException("n must be >= 0 but was " + n);
        }
        if (n < 2) {
            return 1;
        }

        result = recur(n - 1) * n;
        return result;
    }
}

Answer 1

Recursion

It's good that you are trying to grasp recursion, the more programming patterns you understand, the better.

But I would disagree that you need to use recursion more often. Generally, it consumes more memory and iterative solutions tend to be faster as well.

As for your solution, I think that if (n == 1) would be clearer than if (n < 2) (also, n < 2 returns 1 for input 0, which is wrong), and I wouldn't save the return value in result, but return it directly.

Exceptions

In recur throwing an IllegalArgumentException makes sense (it is an illegal argument), but showing the dialog here doesn't make that much sense, I would move that to main.

Also, don't throw an exception you just caught without a reason. If you don't want to execute the following function, just put all code in the try and catch the different exceptions.

Other

• recur isn't a good name. I would call it factorial or factorialRecursive.
• I would wrap the show message code into a function, that way you could also easily exchange it for something else (like a print). If you then further wrap this up in an object, unit testing would also be easier.

Code

If you do all of this, the code would look something like this:

public static void main(String[] args) {
    int factorialNumber = 0;
    try {
        factorialNumber = Integer.parseInt(input("Which number should we compute the factorial of?"));
        print(factorialNumber + " factorial is " + factorialRecursive(factorialNumber) + ".",
                "Result");
    } catch (NumberFormatException nfe) {
        error("Input must be an integer!", "Error");
    } catch (IllegalArgumentException iae) {
        error(iae.getMessage(), "Error");
    }
}

public static int factorialRecursive(int n) {
    if (n < 0) {
        throw new IllegalArgumentException("n must be >= 0 but was " + n);
    }
    if (n < 2) {
        return 1;
    }

    return factorialRecursive(n - 1) * n;
}

public static String input(String message) {
    return JOptionPane.showInputDialog(null, message);
}

public static void print(String message, String title) {
    JOptionPane.showMessageDialog(null,
            message,
            title, JOptionPane.PLAIN_MESSAGE);
}

public static void error(String message, String title) {
    JOptionPane.showMessageDialog(null, message,
            title, JOptionPane.ERROR_MESSAGE);
}

Answer 2

@tim's confusion in his answer indicates that your code could use a comment:

public static int recur(int n) {
    int result;
    if (n < 0) {
        JOptionPane.showMessageDialog(null,"n must be >= 0 but was " + n,
            "Error", JOptionPane.ERROR_MESSAGE);

        throw new IllegalArgumentException("n must be >= 0 but was " + n);
    }
    if (n < 2) {//both 0! and 1! are 1
        return 1;
    }

    result = recur(n - 1) * n;
    return result;
}

I was quite confused myself too.