Critique of Pascal's Triangle Class

I'm working my way through The Java Programming Language, Fourth Edition - The Java Series. This is Exercise 7.3:


Write a program that calculates Pascal's triangle to a depth of 12, storing each row of the triangle in an array of the appropriate length and putting each of the row arrays into an array of 12 int arrays. Design your solution so that the results are printed by a method that prints the array of arrays using the lengths of each array, not a constant 12. Now change the code to use a constant other than 12 without modifying your printing method.

Is the following an adequate solution? Is my use of exception handling reasonable for this use case?

```
/**
* Defines Pascal's Triangle
*/
public class PascalsTriangle {
private final long[][] triangle;

/**
* Sole constructor
*
* @param depth the depth of the triangle
*/
PascalsTriangle(int depth) {
triangle = new long[depth][];
long upperLeft;
long upperRight;

for (int i = 0; i < depth; i++) {
triangle[i] = new long[i + 1];

for (int j = 0; j < i + 1; j++) {
if(i == 0) {
triangle[i][j] = 1; //seed the triangle
}
else {
try {
upperLeft = triangle[i - 1][j - 1];
} catch (ArrayIndexOutOfBoundsException e) {
upperLeft = 0; //upperLeft is 0 if not found
}

try {
upperRight = triangle[i - 1][j];
} catch (ArrayIndexOutOfBoundsException e) {
upperRight = 0; //upperRight is 0 if not found
}

triangle[i][j] = upperLeft + upperRight;
}
}
}
}

/**
* Prints the triangle to System.out
*/
public void printTriangle() {
for (long[] iTriangle : triangle) {
for

There are 2½ things wrong with your code, otherwise it's fine.

-
Use exception handling with try/catch for exceptional circumstances, not for trivial bounds checking which you can do! The snippet

try {
      upperLeft = triangle[i - 1][j - 1];
} catch (ArrayIndexOutOfBoundsException e) {
     upperLeft = 0; //upperLeft is 0 if not found
}

throws an error if j == 0 – you already handled the i == 0 case. You can avoid this by starting the loop one index later, and just putting a 1 in the first field. The same considerations hold for the last array entry.

When we initialize the bounds outside of the loops, we get something like:

// init first row
triangle[0] = new long[]{ 1 };

for (int i = 1; i < depth; i++) {
    triangle[i] = new long[i + 1];

    // init first and last element        
    triangle[i][0] = 1;
    triangle[i][i] = 1;

    for (int j = 1; j < i; j++) {
        long left  = triangle[i - 1][j - 1];
        long right = triangle[i - 1][j    ];
        triangle[i][j] = left + right;
    }
}

By handling these edge cases seperately, I can radically shorten the code, and can omit exception handling.

-
In- and Output should happen in the main method, not in helpers. You have written a printTriangle method, which hardcodes the System.out stream. It would be better to assemble a string representation in a toString() method, and then print out the result from that:

public String toString() {
  Formatter formatter = new Formatter(new StringBuilder(), null);
  for (long[] row : triangle) {
    for (long i : row) {
      formatter.format("%d ", i);
    }
    formatter.format("\n");
  }
  return formatter.toString();
}

Then in main: System.out.print(triangle.toString()).

What I haven't addressed until now is your usage of variables. You should declare your variables as close to their point of usage as possible, and in the tightest possible scope. Your upperLeft etc. is only relevant inside the else branch of your original code – if you look at my version, you see I declared the corresponding variable inside the loop as well.

Oh, and your names for the loop variables in printTriangle() are weird. The letters i and j usually denote integers in loops, a convention taken from mathematics into programming. It is better to use names that are actually connected to the problem you are solving. If you look at my snippet, you'll see that I iterate over each row in the triangle, and then over each long integer i in that row.

StackExchange Code Review Q#32255, answer score: 5