Thread: Pascal's Triangle Program

I wrote a pascal's triangle program with an imbedded 'for' loop and a Factorial class that computes the factorial of a number. When it gets to the third line of the triangle the problems arise. If anyone could help I would greatly appreciate it. Here is my code:

// --------------------------------------------------------------------
// This program is intended to make Pascal's triangle.
// --------------------------------------------------------------------
 
package PascalTriangle;
 
public class PascTri {
	public static void main (String[] args){
		Factorial factorial = new Factorial();
		int n, k, subtract;
		// The number of rows is set to five for now
		int rows = 5;
		long term1;
		long term2;
		long term3;
		long term4;
		long endTerm;
		// 'n' represents the line in Pascal's triangle
		// 'k' represents the term in a line in Pascal's triangle
 
		for(n = 0; n <= rows-1; n++) {
			for(k = 0; k <= n; k++ ) {
				subtract = n-k;
				term1 = factorial.Factorial(n);
				term2 = factorial.Factorial(k);
				term3 = factorial.Factorial(subtract);
				term4 = term2 * term3;
				endTerm = term1/term4;
				System.out.print(endTerm + "   ");
				// I typed this line simply to help troubleshoot
				System.out.print("Term1: " + term1 + " Term2: " + term2 + " Term3: " + term3 + "   |   ");
			}
			System.out.println();
		}
	}
}
 
// **********************************************
// Factorial Class
// **********************************************
class Factorial {
	private long hideFact;
	long i;
	long finFact = 1;
 
	long Factorial(long factNum) {
		hideFact = factNum;
		if (hideFact == 1)
			finFact = 1;
		else if (hideFact == 0)
			finFact = 1;
		else
			for(i = hideFact; i >= 1; i--) {
				finFact *= i;
			}
		return finFact;
	}
}

When it gets to the third line of the triangle the problems arise.

What problems? (Describe both the actual and intended behaviour of your program).

-----

It's a good idea to keep to Java coding conventions: start package names, methods and variables with a lowercase letter and don't give a method the same name as a constructor as you do with Factorial().

And test everything!

public class PascTri {
    public static void main (String[] args){
        Factorial factorial = new Factorial();
        for(int test = 0; test < 10; test++) {
            System.out.printf("%d --> %d%n", test, factorial.Factorial(test));
        }
        int n, k, subtract;
        // etc

Is the factorial method doing what you expect?

Thank you for the small tips.
This program is intended to create a Pascal's triangle, for example:
1
1 1
1 2 1
1 3 3 1
1 4 6 4 1
... etc

That is not the output that is produced. Instead, if this code is run (minus that small println line I typed for testing) this is produced:
1
1 1
1 4 0
1 18 0 0
1 96 0 0 0

When my Factorial class is run separately, it works perfectly. There are no errors whatsoever.

I've tried testing every little part and it seems like the error comes in the imbedded for loop. I had it print all of the values of 'n' throughout the imbedded loop, and they are correct. But when factorial.Factorial(n) is computed for the same value of n, the results are different. I can't figure out why. Can something in my main class affect the Factorial class?

When my Factorial class is run separately, it works perfectly.

Did you try the three lines I added at the start of main()?

The output I get is:

0 --> 1
1 --> 1
2 --> 2
3 --> 12
4 --> 288
5 --> 34560
6 --> 24883200
7 --> 125411328000
8 --> 5056584744960000
9 --> 8705808953839190016

Those numbers are not the factorials, so what exactly is the factorial method supposed to be returning?

I figured it out. When the method Factorial is run just once, the correct value is attained. Because Factorial returns the value of finFact, when Factorial is run again it keeps the previous value of finFact and multiplies that by the correct value. I simply had to change the Factorial class to make finFact = 1 every time. My new Factorial class looks like this:

class Factorial {
	private long hideFact;
	private long i;
	private long finFact = 1;
 
	long doFactorial(long factNum) {
		hideFact = factNum;
		if (hideFact == 0)
			finFact = 1;
		else {
			finFact = 1;
			for(i = hideFact; i >= 1; i--) {
				finFact = finFact * i;
			}
		}	
		return finFact;
	}
}

I changed the name of the method Factorial to doFactorial as you recommended. Thank you so much for your help.

You're welcome - I'm glad you've got it figured out.

In some ways having instance variables can be a menace: as you've found they retain a "memory" from call to call. It's probably best to minimise them.

In the case of doFactorial() there is nothing that has to be "remembered" from call to call. So I'd be looking for a version that used only local variables (ie variables that are declared and used within the doFactorial() method). Of course if you have working code you may not feel inclined to change anything! But I'm just saying that it was having instance variables rather than local variables - specifically finFact - that bit you here.