0

Can't get my program to output the correct number. I feel like I am making a simple mistake. This is written in C.

#include <stdio.h>
#include <stdlib.h>

int main()
   {
    int n, i;
    int list[n];

    while(1)
    {
       scanf("%d", &n);
       if(n == -1)
    {
        break;
    }
    else
     {
        for(i = 2; i < n; i++)
        {
            list[i] = list[i-1]+list[i-2]; 
        }
        printf("%d %d", i, list[i] );
    }
 }
}
5
  • 3
    Your compiler is allowing int list[n]; where n isn't even initialized, let alone constant? Maybe you want to turn your compiler warning level up a bit. You also haven't initialized the base case for the fibonacci sequence (index 0 and 1), so I'm unclear where you think those values are being set. Commented Feb 16, 2017 at 0:10
  • Undefined behavior for using the value of an object with automatic storage duration while it is indeterminate. Commented Feb 16, 2017 at 0:10
  • What output do you get? What do you want it to be? Commented Feb 16, 2017 at 0:12
  • Note that if you just want to output the numbers you do not need to use an array, you only need to hold the most recently calculated 2 values Commented Feb 16, 2017 at 0:34
  • The number @ 5. Now listing, and its needed for this exercise I'm doing for school. They want us to put it in an array. Commented Feb 16, 2017 at 1:02

4 Answers 4

2

(To make things simpler, I'm going to ignore dealing with input.)

First problem is turning on compiler warnings. Most C compilers don't give you warnings by default, you have to ask for them. Usually by compiling with -Wall. Once we do that, the basic problem is revealed.

test.c:6:14: warning: variable 'n' is uninitialized when used here [-Wuninitialized]
    int list[n];
             ^
test.c:5:10: note: initialize the variable 'n' to silence this warning
    int n, i;
         ^
          = 0
1 warning generated.

int list[n] immediately creates a list of size n. Since n is uninitialized it will be garbage. You can printf("%d\n", n); and see, it'll be something like 1551959272.

So either n needs to be initialized, or you need to reallocate list dynamically as n changes. Dynamic allocation and reallocation gets complicated, so let's just make it a static size.

So we get this.

#include <stdio.h>
#include <stdlib.h>

int main() {
    /* Allocate an array of MAX_N integers */
    const int MAX_N = 10;
    int list[MAX_N];

    /* Do Fibonacci */
    for(int i = 2; i < MAX_N; i++) {
        list[i] = list[i-1]+list[i-2]; 
    }

    /* Print each element of the list and its index */
    for( int i = 0; i < MAX_N; i++ ) {
        printf("%d\n", list[i]);
    }
}

That runs, but we get nothing but zeros (or garbage). You have a problem with your Fibonacci algorithm. It's f(n) = f(n-1) + f(n-2) with the initial conditions f(0) = 0 and f(1) = 1. You don't set those initial conditions. list is never initialized, so list[0] and list[1] will contain whatever garbage was in that hunk of memory.

#include <stdio.h>
#include <stdlib.h>

int main() {
    /* Allocate an array of MAX_N integers */
    const int MAX_N = 10;
    int list[MAX_N];

    /* Set the initial conditions */
    list[0] = 0;
    list[1] = 1;

    /* Do Fibonacci */
    for(int i = 2; i < MAX_N; i++) {
        list[i] = list[i-1]+list[i-2]; 
    }

    /* Print each element of the list and its index */
    for( int i = 0; i < MAX_N; i++ ) {
        printf("%d\n", list[i]);
    }
}

Now it works.

0 0
1 1
2 1
3 2
4 3
5 5
6 8
7 13
8 21
9 34
Sign up to request clarification or add additional context in comments.

4 Comments

Your last piece of code says "list[0] is already initialized to 0" , but it never was. "Stack allocated arrays get initialized to 0" is false
@M.M Stack allocated lists get initialized to 0, or does it need an explicit {}?
@M.M I think you're right. "If an object that has automatic storage duration is not initialized explicitly, its value is indeterminate."
Awesome explanation. I hope to really understand C more as I continue learning. I always seem to hit a block in my programming where I feel like I'm getting nowhere with any of my programs. Thank you so much. I'm going to read through this a few times to get a good understanding.
0

Here is code snippet,

#include <stdio.h>
int main()
{
    int MAX_SIZE = 100; //Initial value
    int n, i;
    int list[MAX_SIZE];

    printf("Enter value of 'n'");
    scanf("%d",&n);

    if(n < 0){
       printf("'n' cannot be negative number");
       return 0;
    }else if (n==1){
       list[0]=0;
    }else if(n == 2){
       list[0]=0;
       list[1]=1;
    }else{
        list[0]=0;
        list[1]=1;
        for(i = 2; i <= n; i++)
        {
          list[i] = list[i-1]+list[i-2]; 
        }
    }
    //To view array elements
    for(int i=0;i<n;i++){
        printf("%3d",list[i]);
    }

   }

4 Comments

You should also deal with n exceeding the array size
You should also deal with list[i] exceeding the capacity of an int. Setting MAX_SIZE to 47 might be a good start.
That is true, need to add extra logic to deal with it ( allocation of memory for array should change dynamically)
Ended up using doubles anyways
0

You don't have return in main function.

n must be defined previous. Otherwise it took random value from memory. So, your list array is created with unknown value.

int list[n];

Also, this will never happends, becous n is declared, but not defined.

i < n;

Is this what you need?

#include <stdio.h>
#include <stdlib.h>

int main()
{
    int F[100];
    F[0] = 0;
    F[1] = 1;
    int i = 2;
    while(1)
    {
        if(i < 100)
        {
            F[i] = F[i-1] + F[i-2];
            i++;
        }
        else
        {
            break;
        }
    }
    i = 0;
    while(1)
    {
        if(i < 100)
        {
            printf("%d ; ", F[i]);
            i++;
        }
        else
        {
            break;
        }
    }
    return 0;
}

1 Comment

It's not necessary to return in main. From 5.1.2.2.3 of ISO C: "reaching the } that terminates the main function returns a value of 0."
0

You need to allocate memory on demand for each iteration. In your code, n is uninitalized which leads to unpredectiable behavior. Also you need to initialize list[0] and list[1] since this is the 'base' case.

#include <stdio.h>
#include <stdlib.h>

int main()
{
  int n, i;
  int* list; /* Declare a pointer to the list */

  while(1)
  {
     scanf("%d", &n);
     if(n == -1)
     {
       break;
     }
     else if ( n > 0 )
     {
       list = (int *) malloc( n * sizeof(int) );
       list[0] = 1;
       list[1] = 1;
       for(i = 2; i < n; i++)
       {
         list[i] = list[i-1]+list[i-2]; 
       }
       printf("%d %d\n", i, list[i-1] );
       free(list);
     }
   }
}

Comments

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.