I created a Pascal's triangle. I'm in 2nd semester, so I'm not that professional in programming. If there are some suggestions for improvement, please tell (took me 2 & 1/2 hrs to find the logic).
//Pascals Triangle
package logicBuilding;
import java.util.*;
public class PascalsTriangle {
static void print(int[] arr,int r) {
int size=2,t=1,n=0;
int [] arr1=null;
int temp1=1,temp2=1,sizeArr1=2;
for (int i=0;i<r;i++) {
for (int k = r - i - 1; k > 0; k--) {
System.out.print(" ");
}
for (int j = 0; j <= i; j++) {
if (i == 0) {
System.out.print(arr[i]);
break;
} else if (i == 1) {
System.out.print(arr[i - 1] + " " + arr[i]);
break;
}
System.out.print(arr[j] + " ");
}
if(i > 2) {
arr1 = new int[sizeArr1];
for(int a=1;a<size-1;a++) {
arr1[n]=arr[a];
n++;
}
n=0;
}
if (i > 0) {
arr = new int[++size];
arr[1] = ++temp1;
arr[size - 2] = ++temp2;
arr[size - 1] = 1;
arr[0] = 1;
}
if (i > 2) {
for (int g = 0; g < sizeArr1; g++) {
if (t < sizeArr1) {
arr1[g] += arr1[t];
t++;
} else {
break;
}
}
int w = 0;
while (w < sizeArr1) {
for (int h = 2; h < size - 2; h++) {
arr[h] = arr1[w];
w++;
}
break;
}
t = 1;
sizeArr1++;
}
System.out.println("");
}
}
public static void main(String[] args) {
Scanner s1=new Scanner(System.in);
int [] arr;
int r;
System.out.println("Enter the number of rows you want : ");
r=s1.nextInt();
s1.nextLine();
arr=new int[2];
arr[0]=1;
arr[1]=1;
print(arr,r);
}
}
3 Answers 3
Avoid importing *
- it pollutes your namespace.
The structure of the program is not very Java-idiomatic. The typical Java pattern would be to instantiate an object that represents a triangle of a certain size.
You should separate your calculation code from your rendering (print()
) code.
In lines like print(arr[i - 1] + " " + arr[i])
, you should avoid concatenation and make better use of formatting.
Consider:
- Making an object instance with a
final nrows
to represent your triangle - Making an iterator object to generate your rows
- Making a
render
method that accepts aPrintStream
Suggested
package com.stackexchange;
import java.io.PrintStream;
import java.util.Arrays;
import java.util.Iterator;
import java.util.Scanner;
import java.util.stream.Collectors;
public class PascalsTriangle {
public class RowIterator implements Iterator<int[]>, Iterable<int[]> {
private int n = 0;
@Override
public Iterator<int[]> iterator() {
return new RowIterator();
}
@Override
public boolean hasNext() {
return n < nrows;
}
@Override
public int[] next() {
int[] row = new int[n + 1];
int nck = 1;
for (int k = 0; k <= n; ++k) {
row[k] = nck;
nck = nck*(n - k)/(k + 1);
}
++n;
return row;
}
}
public final int nrows;
public static int element(int n, int k) {
// n! / k!(n - k)!
int x = 1;
for (int i = 1+k; i <= n; ++i)
x *= i;
for (int i = n-k; i > 1; --i)
x /= i;
return x;
}
public int maxElement() {
return element(nrows - 1, nrows/2);
}
public PascalsTriangle(int nrows) {
this.nrows = nrows;
}
public Iterable<int[]> rows() {
return new RowIterator();
}
public void render(PrintStream out) {
int width = 1 + (int)Math.floor(Math.log10(maxElement()));
String format = "%%%dd".formatted(width);
for (int[] row: rows()) {
String rowstr = Arrays.stream(row)
.mapToObj(format::formatted)
.collect(Collectors.joining(" "));
out.print(" ".repeat(
((width+1)*nrows - rowstr.length())/2
));
out.println(rowstr);
}
}
public static PascalsTriangle fromConsole() {
Scanner scanner = new Scanner(System.in);
System.out.print("Enter the number of rows you want: ");
int r = scanner.nextInt();
scanner.nextLine();
return new PascalsTriangle(r);
}
public static void main(String[] args) {
PascalsTriangle tri = PascalsTriangle.fromConsole();
tri.render(System.out);
}
}
Enter the number of rows you want: 18
1
1 1
1 2 1
1 3 3 1
1 4 6 4 1
1 5 10 10 5 1
1 6 15 20 15 6 1
1 7 21 35 35 21 7 1
1 8 28 56 70 56 28 8 1
1 9 36 84 126 126 84 36 9 1
1 10 45 120 210 252 210 120 45 10 1
1 11 55 165 330 462 462 330 165 55 11 1
1 12 66 220 495 792 924 792 495 220 66 12 1
1 13 78 286 715 1287 1716 1716 1287 715 286 78 13 1
1 14 91 364 1001 2002 3003 3432 3003 2002 1001 364 91 14 1
1 15 105 455 1365 3003 5005 6435 6435 5005 3003 1365 455 105 15 1
1 16 120 560 1820 4368 8008 11440 12870 11440 8008 4368 1820 560 120 16 1
1 17 136 680 2380 6188 12376 19448 24310 24310 19448 12376 6188 2380 680 136 17 1
cite your reference
(took me 2 & 1/2 hrs to find the logic)
Please don't make your reviewers and maintenance engineers go hunting for the reference you eventually found. Include the URL in the source code as a comment.
Hopefully there is some correspondence between the identifiers you're using and what appears in that reference.
javadoc
There isn't any.
Please attach at least a one-sentence /** description */
to the PascalsTriangle
class.
main
This is an OK local variable name.
Scanner s1 = new Scanner(System.in);
It suggests there's a second Scanner somewhere, but there isn't.
Consider using letters instead, one of {sc
, scn
, scnr
, scanner
}.
design of API
This function is well named. It's pretty clear that we shall evaluate it for side effects.
static void print(int[] arr, int r) {
It is unclear why caller is responsible for passing
in a (1, 1)
tuple.
Shouldn't this routine be responsible for creating arr
?
If there is some design rationale, write a comment describing it.
The identifier r
is manifestly the wrong name.
It's not like this is a local variable, so the documentation burden
is much greater. You are responsible for explaining the meaning
of this parameter to an engineer who is calling into this routine.
Name it something like numRows
.
local variables
int size=2, t=1, n=0;
int [] arr1=null;
int temp1=1, temp2=1, sizeArr1=2;
The i
, j
are fine.
Otherwise these are not terrific name choices.
Imagine you have to return to this code in six months and maintain it.
You will wish you were staring at more helpful names.
It's unclear why inventing the concept of sizeArr1
is even helpful;
we could have just allocated a (non-null) integer array of size 2
and then relied on arr1.length
.
I suspect that the concept modeled by some of these is
actually i
, that is, currentRowNum
.
Try to use identifiers drawn from the Business Domain when feasible.
Here, I was expecting to maybe see a coeff
vector
full of numRows
binomial coefficients.
Not sure if arr
or arr1
is supposed to play that role.
Inventing local temp vars like temp1
and temp2
can be fine.
But strive to keep the scope so small that they're self-explanatory.
Here, they're set to unity, and then you're asking a
maintenance engineer to keep those cryptic numbers in mind
until way down below.
The bigger the scope, the greater your responsibility
to explain what's going on, typically via an informative name.
extract helper
for (int k = r - i - 1; k > 0; k--) {
System.out.print(" ");
}
Aha! Now here is some wonderful code. It's clear what it does.
But this loop appears alongside some bigger loops,
and it's a little distracting.
Replacing it with a single-line
System.out.print(indent(numRows - i - 1))
would be nicer.
And then that helper could use StringBuilder or whatever.
display of each numeric cell
You might want to take advantage of the ^
caret
formatting
character, to help with centering each displayed coefficient
within a fixed-width cell.
In the current code,
log_10 of a coefficient influences the horizontal layout.
murky logic
for (int j = 0; j <= i; j++) {
if (i == 0) { ...
} else if (i == 1) { ...
}
if (i > 2) { ...
It's unclear why the first two rows should be so special.
And then we encounter the general case, but oddly it is
outside the loop.
Consider turning that for j
loop into a helper,
just so you can name it, explaining what it accomplishes.
Yes, I know this is a "print" function.
But I'm a little surprised it doesn't first produce
an array containing a prevRow
or currRow
of binomial coefficients,
ideally by calling a helper.
With the array in hand, the print()
routine could focus on its
single responsibility
of formatting those numbers on stdout.
loop index
for (int g = 0; g < sizeArr1; g++) {
We strayed from i, j, ..., that's fine.
But now I'm scratching my head what g
might possibly stand for.
Either explain it, or stick with boring index names.
Consider extracting this for
loop as a tiny helper,
which would give you the opportunity to name what it does.
At least with h
& w
we get the clue to mentally pronounce
them as "height" & "width" of some diagram that appears
in the cited reference.
unit tests
There aren't any, partly due to the lack of any functional helpers. A function that is focused on displaying a result, rather than returning a result object, tends to be one that isn't a great fit for unit tests.
Writing automated tests can save on edit-debug cycle time. But it can also save time spent developing the larger project, by encouraging us to break big problems into smaller ones that are easy to describe to colleagues.
This code appears to achieve its design goal.
I would not be willing to delegate or accept maintenance tasks on this code in its current form.
Code reviewed from a non-math perspective with just code comments.
Efficiency and software development principles (e.g. single responsibility principle a.k.a SRP)
//Pascals Triangle
package logicBuilding;
import java.util.*;
public class Triangle {
static void print(int r) { // pass just the number of lines to print method anything else is internal logic of print method
int[] arr = { 1, 1 }; // use inline array initialization
int size = 2, t = 1, n = 0;
int[] arr1 = null;
int temp1 = 1, temp2 = 1, sizeArr1 = 2;
for (int i = 0; i < r; i++) {
for (int k = r - i - 1; k > 0; k--) {
System.out.print(" ");
}
for (int j = 0; j <= i; j++) {
if (i == 0) { // efficiency wise
System.out.print(arr[i]);
break;
} else if (i == 1) {
System.out.print(arr[0] + " " + arr[1]); // i = 1 use static values for array index, 0 and 1
break;
}
System.out.print(arr[j] + " ");
}
if (i > 2) {
arr1 = new int[sizeArr1];
for (int a = 1; a < size - 1; a++) {
arr1[n] = arr[a];
n++;
}
n = 0;
}
if (i > 0) {
arr = new int[++size];
arr[1] = ++temp1;
arr[size - 2] = ++temp2;
arr[size - 1] = 1;
arr[0] = 1;
}
if (i > 2) {
for (int g = 0; g < sizeArr1; g++) {
if (t < sizeArr1) {
arr1[g] += arr1[t];
t++;
} else {
break;
}
}
int w = 0;
while (w < sizeArr1) {
for (int h = 2; h < size - 2; h++) {
arr[h] = arr1[w];
w++;
}
break;
}
t = 1;
sizeArr1++;
}
System.out.println("");
}
}
public static void main(String[] args) {
Scanner s1 = new Scanner(System.in);
System.out.println("Enter the number of rows you want : ");
int r = s1.nextInt(); // use inline variable declaration initialization
s1.close(); // close the scanner
print(r);
}
}
For improved readability use meaningful variable names
//Pascals Triangle
package logicBuilding;
import java.util.*;
public class Triangle {
static void print(int rowsNumber) { // rowsNumber than r
int[] row = { 1, 1 }; // row than arr
int rowLength = 2, t = 1, n = 0;
int[] arr1 = null;
int temp1 = 1, temp2 = 1, sizeArr1 = 2;
for (int rowNumber = 0; rowNumber < rowsNumber; rowNumber++) { // rowNumber than i
for (int column = rowsNumber - rowNumber - 1; column > 0; column--) { // column than k
System.out.print(" ");
}
for (int j = 0; j <= rowNumber; j++) {
if (rowNumber == 0) {
System.out.print(row[rowNumber]);
break;
} else if (rowNumber == 1) {
System.out.print(row[0] + " " + row[1]);
break;
}
System.out.print(row[j] + " ");
}
if (rowNumber > 2) {
arr1 = new int[sizeArr1];
for (int a = 1; a < rowLength - 1; a++) {
arr1[n] = row[a];
n++;
}
n = 0;
}
if (rowNumber > 0) {
row = new int[++rowLength];
row[1] = ++temp1;
row[rowLength - 2] = ++temp2;
row[rowLength - 1] = 1;
row[0] = 1;
}
if (rowNumber > 2) {
for (int g = 0; g < sizeArr1; g++) {
if (t < sizeArr1) {
arr1[g] += arr1[t];
t++;
} else {
break;
}
}
int w = 0;
while (w < sizeArr1) {
for (int h = 2; h < rowLength - 2; h++) {
row[h] = arr1[w];
w++;
}
break;
}
t = 1;
sizeArr1++;
}
System.out.println("");
}
}
public static void main(String[] args) {
Scanner input = new Scanner(System.in); // input than s1
System.out.println("Enter the number of rows you want: ");
int rowsNumber = input.nextInt(); // rowsNumber than r
input.close();
print(rowsNumber);
}
}
This review is a hint that in case is helpful it should be scaled to cover the entire code.