4
public static void main(String[] args) {

    int n = factorial(30);
    int x = 0;
    while (x <= 30) {
        System.out.println(x + " " + n);
        x = x + 1;
    }


    public static int factorial (int n) {   
       if (n == 0) {
             return 1;
        } else {
            return n * factorial (n-1);
        }
    }
} 

I'm trying to print out something like this:

0 1
1 1
2 2
3 6
4 24
...etc, up to 30 (30!)

What I'm getting instead is this:

0 (30!)
1 (30!)
...etc, up to 30

In words, I'm able to create the left column from 0 to 30 but I want to make it print the factorial of the numbers in the right hand column. With my code, it only prints the factorial of 30 in the right-hand column. I want it to print the factorials in order next to their corresponding number. How can I fix my code to do this?

Nic
  • 6,211
  • 10
  • 46
  • 69
panzo
  • 61
  • 1
  • 6
  • Why not change `n` inside of your while loop and before printing its value? – Hovercraft Full Of Eels Apr 27 '15 at 22:07
  • possible duplicate of [Recursion and the Return Keyword](http://stackoverflow.com/questions/29903226/recursion-and-the-return-keyword) – ericbn Apr 27 '15 at 22:11
  • 1
    SO is not the appropriate forum for homework. – Andy Jones Apr 27 '15 at 22:15
  • @AndyJones But it is the place for people with specific, answerable programming questions -- which this is. It's not a duplicate of another question because it came from the same source. – Nic Apr 27 '15 at 22:17
  • @AndyJones it's not wrong asking questions from solving homework exercises. What's wrong is asking for solving the homework, but this isn't the case. – Luiggi Mendoza Apr 27 '15 at 22:21
  • I did most of it myself I'm just having a small problem with a piece of it. By the way @QPaysTaxes thank you for the help. – panzo Apr 27 '15 at 22:37

2 Answers2

9

This is pretty simple. Instead of defining a variable, you call the method with the updated x every time:

System.out.println(x + " " + factorial(x));

Note that your loop could be rewritten as a for loop, which is exactly what they're designed for:

for (int x = 0; x < 30; x++) {
    System.out.println(x + " " + factorial(x));
}

Note a couple of things:

  1. The x++. It's basically a short form of x = x + 1, though there are some caveats. See this question for more information about that.
  2. x is defined in the loop (for (int x = ...) not before it
  3. n is never defined or used. Rather than setting a variable that's only used once, I directly used the result of factorial(x).

Note: I'm actually pretty certain that an int will overflow when confronted with 30!. 265252859812191058636308480000000 is a pretty big number. It also overflows long, as it turns out. If you want to handle it properly, use BigInteger:

public BigInteger factorial(int n) {
    if (n == 0) {
        return BigInteger.ONE;
    } else {
        return new BigInteger(n) * factorial(n - 1);
    }
}

Because of BigInteger#toString()'s magic, you don't have to change anything in main to make this work, though I still recommend following the advice above.

Community
  • 1
  • 1
Nic
  • 6,211
  • 10
  • 46
  • 69
0

As @QPaysTaxes explains, the issue in your code was due to computing the final value and then printing it repeatedly rather than printing each step.

However, even that working approach suffers from a lack of efficiency - the result for 1 computes the results for 0 and 1, the result for 2 computes the results for 0, 1, and 2, the result for 3 computes the results for 0, 1, 2, and 3, and so on. Instead, print each step within the function itself:

import java.math.BigInteger;

public class Main
{

    public static BigInteger factorial (int n) {   
        if (n == 0) {
            System.out.println("0 1");
            return BigInteger.ONE;
          } else {
            BigInteger x = BigInteger.valueOf(n).multiply(factorial(n - 1));
          System.out.println(n + " " + x);
            return x;
          }
      }
  public static void main(String[] args)
  {
    factorial(30);
  }
}

Of course, it would be faster and simpler to just multiply in the loop:

import java.math.BigInteger;

public class Main
{
  public static void main(String[] args)
  {
    System.out.println("0 1");
    BigInteger y = BigInteger.ONE;
    for (int x = 1; x < 30; ++x) {
        y = y.multiply(BigInteger.valueOf(x));
        System.out.println(x + " " + y);
    }
  }
}

Just for fun, here's the efficient recursive solution in Python:

def f(n):
    if not n:
        print(0, 1)
        return 1
    else:
        a = n*f(n-1)
        print(n, a)
        return a

_ = f(30)

And, better still, the iterative solution in Python:

r = 1
for i in range(31):
    r *= i or 1
    print(i, r)
Community
  • 1
  • 1
TigerhawkT3
  • 48,464
  • 6
  • 60
  • 97