HiveBrain v1.2.0
Get Started
← Back to all entries
patterncMinor

Pascal Triangle program in C

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
pascalprogramtriangle

Problem

I've created a program to calculate Pascal's triangle:

#include 
#include 
void pascal(int limit)
{
    int *begin;
    begin=(int*)malloc(2*sizeof(int));
    *begin=1;
    *(begin+1)=1;
    int* p,q,s,t;
    int row=2,n=3,x;
    p=begin;
    q=begin+1;
    for(n=3;n<=limit+1;n++)
    {
        if(n==4)
        {
            free(begin);
        }
        x=1;
        printf("Row (%d) : ",row);
        for(int* i=p;i<=q;i=i+1)
        {
            printf("%d ",*i);
        }
        putchar('\n');
        s=(int*)malloc(n*sizeof(int));
        t=s+n-1;
        *s=*p;
        *t=*q;
        for(int* g=p;g<q;g=g+1)
        {
            *(s+x)=*g+*(g+1);
            x++;
        }
        p=s;
        q=t;
        row++;
    }
}

int main(void)
{
    int i;
    printf("Till which row would you like to calculate the Pascal Triangle?\n\n");
    printf("Type your answer:\t");
    scanf("%d",&i);
    putchar('\n');
    if(i<n)
        printf("Nope, you can't do that\n");
    else
    {
        printf("Row (1) : 1\n");
        pascal(i);
    }
}


My idea was to work with 2 arrays at the same time, but there is still a problem in my algorithm. How am I supposed to free the memory I allocated before? The first 8 bytes of begin are easy to free, but what should I do so I can free the memory I allocated through the pointers?

Solution

-
you don't free s which means you have a memory leak

-
*(begin+1) can be rewritten as begin[1], which is much clearer

-
1 char variable names are very hard to get into, use longer/descriptive names where needed

the entire function can be rewritten as

void pascal(int limit)
{

    int *row;
    row=(int*)malloc(2*sizeof(int));
    row[0]=1;
    row[1]=1;

    for(int n=3;n<=limit+1;n++){

    printRow(buffer,n);

    int* nextRow = (int*)malloc((1+n)*sizeof(int));
    nextRow[0]=row[0];
    nextRow[n]=row[n-1];

    for(int i=1;i<n;i++){
        nextRow[i]=row[i-1]+row[i];
    }
    free(row);
    row=nextRow;

    }
}


instead of the malloc->assign->free sequence I could have used realloc and then worked backwards:

row = (int*)realloc(row,(1+n)*sizeof(int));
row[n]=0;

for(int i=n;i>0;i--){
    row[i]+=row[i-1];
}


but this is less clear

Code Snippets

void pascal(int limit)
{

    int *row;
    row=(int*)malloc(2*sizeof(int));
    row[0]=1;
    row[1]=1;

    for(int n=3;n<=limit+1;n++){

    printRow(buffer,n);

    int* nextRow = (int*)malloc((1+n)*sizeof(int));
    nextRow[0]=row[0];
    nextRow[n]=row[n-1];

    for(int i=1;i<n;i++){
        nextRow[i]=row[i-1]+row[i];
    }
    free(row);
    row=nextRow;

    }
}
row = (int*)realloc(row,(1+n)*sizeof(int));
row[n]=0;

for(int i=n;i>0;i--){
    row[i]+=row[i-1];
}

Context

StackExchange Code Review Q#44330, answer score: 5

Revisions (0)

No revisions yet.