Review C Code: Find & Resolve Issues in Visual Studio & repl.it

[doktorwho]

Hi, i wrote a code and annotated it but i have few problems with it. First it won't run on Visual Studio but it runs fine on repl.it online site which uses a C ide. Second, it seems wrong when i input
5
45 6 -4 8 3
It should print out 8 3 but for some reason it misses out on 3. Could you see what's wrong with it and give me a hint on how to resolve?

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

//Program that from the numbers in the array searches for non-fibonacci numbers and then deletes them from the list.
void main() {
    int MAX_NUM_EL = 500;
    int numbers[MAX_NUM_EL];
    int i, n;
    int k, j, f0 = 0, f1 = 1, fib = 1;

    printf("Enter the size of your array: \n");
    scanf("%d", &n);
    if (n>MAX_NUM_EL || n<0) {
        printf("You've typed an invalid length of the array\n");
        return 0;
    }
    if (n == 0) {
        printf("Your array has no elements to check. The program will exit now\n");
        return 0;
    }

    printf("Enter the numbers: \n");
    for (i = 0; i < n; i++)
        scanf("%d", &numbers[i]);    for (i = 0; i < n; i++) {
        if (numbers[i] < 0) {                       //Checking for negative numbers.
            for (k = i; k < n; k++) {               //If it find a negative number it deletes it by placing the forward element.
                numbers[k] = numbers[k + 1];        //into it's place and reduces the number of elements by 1.
            }
            n--;
            i--;
            continue;
        }

        if (numbers[i] == 0)
            continue;

        for (j = 0; j < numbers[i]; j++) {
            if (numbers[i] == fib)
                continue;
            f0 = f1;
            f1 = fib;
            fib = f0 + f1;                      //It goes through all the fibonacci numbers to see if that element fits.
            if (numbers[i] < fib) {
                for (k = i; k < n; k++) {          //If we reach fibonacci that is bigger than that element does fit and we remove it.
                    numbers[k] = numbers[k + 1];
                }
                n--;
                i--;
                f0 = 0;
                f1 = 1;
                fib = 1;
                continue;
            }
        }
    }
    printf("This is the resulting array: \n");
    for (i = 0; i < n; i++)
        printf(" %d ", numbers[i]);

    return 0;
}

 

[ChrisVer]

First, what problem do you get from VS?

Second, when you throw out the negative numbers it would be much easier to follow if you don't change the range of your iteration (n--) but your index instead to account for the missing element. Also one way to do some bug-hunting would be to introduce cout statements within your code, it will help you find where things go wrong.
Also what does it do for 8 3 ? does it still miss the 3?

 

[doktorwho]

First, what problem do you get from VS?

Second, when you throw out the negative numbers it would be much easier to follow if you don't change the range of your iteration (n--) but your index instead to account for the missing element. Also one way to do some bug-hunting would be to introduce cout statements within your code, it will help you find where things go wrong.

It turns out visual studio works if i change every return 0; to exit(0); and i also made some slight changes to the code which now looks like this:

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

//Program that from the numbers in the array searches for non-fibonacci numbers and then deletes them from the list.
void main() {
    int MAX_NUM_EL = 500;
    int numbers[MAX_NUM_EL];
    int i, n;
    int k, j, f0 = 0, f1 = 1, fib = 1;

    printf("Enter the size of your array: \n");
    scanf("%d", &n);
    if (n>MAX_NUM_EL || n<0) {
        printf("You've typed an invalid length of the array\n");
        exit(0);
    }
    if (n == 0) {
        printf("Your array has no elements to check. The program will exit now\n");
        exit(0);
    }

    printf("Enter the numbers: \n");
    for (i = 0; i < n; i++)
        scanf("%d", &numbers[i]);    for (i = 0; i < n; i++) {
        if (numbers[i] < 0) {                       //Checking for negative numbers.
            for (k = i; k < n; k++) {               //If it find a negative number it deletes it by placing the forward element.
                numbers[k] = numbers[k + 1];        //into it's place and reduces the number of elements by 1.
            }
            n--;
            i--;
            continue;
        }

        if (numbers[i] == 0)
            continue;

        for (j = 0; j < numbers[i]; j++) {
            if (numbers[i] == fib) {
            f0 = 0;
            f1 = 1;
            fib = 1;
                continue;
            }
            f0 = f1;
            f1 = fib;
            fib = f0 + f1;                      //It goes through all the fibonacci numbers to see if that element fits.
            if (numbers[i] < fib) {
                for (k = i; k < n; k++) {          //If we reach fibonacci that is bigger than that element does fit and we remove it.
                    numbers[k] = numbers[k + 1];
                }
                n--;
                i--;
                f0 = 0;
                f1 = 1;
                fib = 1;
                continue;
            }
        }
    }
    printf("This is the resulting array: \n");
    for (i = 0; i < n; i++)
        printf(" %d ", numbers[i]);

    exit(0);
}

I do not know yet if it works but I am going to do some simplifications if possible. Hmm i don't follow your n-- argument. Could you explain a little more?

 

[ChrisVer]

I mean try to input the array 8 3 and see what the output is... then try also 3 8... maybe something goes wrong with the assignments of f0,f1 once you have a decrease to the numbers in your array (since they no longer have the starting values of 0,1 in the next number's iteration).
As for the n--, I think I might be wrong there... but for example I would refrain from changing the array I am given into the final result and instead try to create a new one from the elements of the input one. So instead of manipulating the array itself within the loop, manipulate an copy of it let's say (but that's useful for example in python lists, maybe your program doesn't have a problem with it).

 

[haruspex]

I am not entirely clear on the spec. Is it to treat each number independently and decide whether it fits in the standard Fibonacci sequence, or do you require the retained numbers to be in order within the sequence? If each is independent then the method seems strangely complicated.
(You could build a chain of Fibonacci numbers and scan down it for each presented number. If the presented number exceeds the last in the chain then extend the chain dynamically as needed. More efficient methods are possible which combine arrays with chains. That would allow an extensible structure while also letting you binary chop to find the match - or mismatch - faster.)

As regards debugging, how about finding a simpler failure case, just 8 3 possibly, then stepping through the algorithm by hand, playing the role of the computer?

 

[haruspex]

visual studio works if i change every return 0; to exit(0);

Sounds like you need a flush call to push out the last of the print queue.

 

[Aufbauwerk 2045]

I don't think it will be helpful to correct it myself, but here are a couple of test runs. I used gcc from mingw32 to compile since I didn't want to take the time to set up a VC++ project.

As you know, the first Fibonacci numbers are 0, 1, 1, 2, 3, 5, 8, 13, 21, 34, 55 which covers the following tests.

Here is your example.

Enter the size of your array:
5
Enter the numbers:
45
6
-4
8
3
This is the resulting array:

nothing!

Here is your example again, but I've entered the numbers in order.

Enter the size of your array:
5
Enter the numbers:
-4
3
6
8
45
This is the resulting array:
3 8

So at least in this case, your program gives the right answer if I sort the numbers first.

 

[Aufbauwerk 2045]

Here's an easy simplification. The only output of your program is the printout of the Fibonacci numbers in the array. So just test each number in turn and print it right away if it's a Fibonacci number. You don't need to change the array itself.

 

[Mark44]

Hi, i wrote a code and annotated it but i have few problems with it. First it won't run on Visual Studio but it runs fine on repl.it online site which uses a C ide. Second, it seems wrong when i input
5
45 6 -4 8 3
It should print out 8 3 but for some reason it misses out on 3. Could you see what's wrong with it and give me a hint on how to resolve?

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

//Program that from the numbers in the array searches for non-fibonacci numbers and then deletes them from the list.
void main() {
    int MAX_NUM_EL = 500;
    int numbers[MAX_NUM_EL];
    int i, n;
    int k, j, f0 = 0, f1 = 1, fib = 1;

    printf("Enter the size of your array: \n");
    scanf("%d", &n);
    if (n>MAX_NUM_EL || n<0) {
        printf("You've typed an invalid length of the array\n");
        return 0;
    }
    if (n == 0) {
        printf("Your array has no elements to check. The program will exit now\n");
        return 0;
    }

    printf("Enter the numbers: \n");
    for (i = 0; i < n; i++)
        scanf("%d", &numbers[i]);    for (i = 0; i < n; i++) {
        if (numbers[i] < 0) {                       //Checking for negative numbers.
            for (k = i; k < n; k++) {               //If it find a negative number it deletes it by placing the forward element.
                numbers[k] = numbers[k + 1];        //into it's place and reduces the number of elements by 1.
            }
            n--;
            i--;
            continue;
        }

        if (numbers[i] == 0)
            continue;

        for (j = 0; j < numbers[i]; j++) {
            if (numbers[i] == fib)
                continue;
            f0 = f1;
            f1 = fib;
            fib = f0 + f1;                      //It goes through all the fibonacci numbers to see if that element fits.
            if (numbers[i] < fib) {
                for (k = i; k < n; k++) {          //If we reach fibonacci that is bigger than that element does fit and we remove it.
                    numbers[k] = numbers[k + 1];
                }
                n--;
                i--;
                f0 = 0;
                f1 = 1;
                fib = 1;
                continue;
            }
        }
    }
    printf("This is the resulting array: \n");
    for (i = 0; i < n; i++)
        printf(" %d ", numbers[i]);

    return 0;
}

You have declared main() as a void function, which means that it is an error to have a statement of the form "return <some number>". The easy fix is to declare main() as "int main(void)". VS correctly noted the error in your code, the other software did not. It's not good programming practice to declare main() as a void function.

 

[doktorwho]

I mean try to input the array 8 3 and see what the output is... then try also 3 8... maybe something goes wrong with the assignments of f0,f1 once you have a decrease to the numbers in your array (since they no longer have the starting values of 0,1 in the next number's iteration).
As for the n--, I think I might be wrong there... but for example I would refrain from changing the array I am given into the final result and instead try to create a new one from the elements of the input one. So instead of manipulating the array itself within the loop, manipulate an copy of it let's say (but that's useful for example in python lists, maybe your program doesn't have a problem with it).

I am not entirely clear on the spec. Is it to treat each number independently and decide whether it fits in the standard Fibonacci sequence, or do you require the retained numbers to be in order within the sequence? If each is independent then the method seems strangely complicated.
(You could build a chain of Fibonacci numbers and scan down it for each presented number. If the presented number exceeds the last in the chain then extend the chain dynamically as needed. More efficient methods are possible which combine arrays with chains. That would allow an extensible structure while also letting you binary chop to find the match - or mismatch - faster.)

As regards debugging, how about finding a simpler failure case, just 8 3 possibly, then stepping through the algorithm by hand, playing the role of the computer?

Here's an easy simplification. The only output of your program is the printout of the Fibonacci numbers in the array. So just test each number in turn and print it right away if it's a Fibonacci number. You don't need to change the array itself.

I thought about printing out the numbers that are in agreement with the fibonacci sequence or creating a new array but the problem specifically states that the numbers are to be checked and if they don't comply are erased from the original array. As for your dynamic sequence I'm afraid that's currently beyond the scope of my abilities as I've just started learning C. Maybe you can provide some link for me to educate myself nd try to correct the code?

 

[doktorwho]

I don't think it will be helpful to correct it myself, but here are a couple of test runs. I used gcc from mingw32 to compile since I didn't want to take the time to set up a VC++ project.

As you know, the first Fibonacci numbers are 0, 1, 1, 2, 3, 5, 8, 13, 21, 34, 55 which covers the following tests.

Here is your example.

Enter the size of your array:
5
Enter the numbers:
45
6
-4
8
3
This is the resulting array:

nothing!

Here is your example again, but I've entered the numbers in order.

Enter the size of your array:
5
Enter the numbers:
-4
3
6
8
45
This is the resulting array:
3 8

So at least in this case, your program gives the right answer if I sort the numbers first.

I fixed the algortihm and it works good now. It fixed the issue of if numbers that are fibonacci come first the former are not displayed. The problem was that i didn't reset the fib number and it stayed at the value of the first fib.
Here it is:

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

//Program that from the numbers in the array searches for non-fibonacci numbers and then deletes them from the list.
int main(void) {
    static const int MAX_NUM_EL = 500;
    int numbers[MAX_NUM_EL];
    int i, n;
    int k, j, f0 = 0, f1 = 1, fib = 1;

    printf("Enter the size of your array: \n");
    scanf("%d", &n);
    if (n>MAX_NUM_EL || n<0) {
        printf("You've typed an invalid length of the array\n");
        exit(0);
    }
    if (n == 0) {
        printf("Your array has no elements to check. The program will exit now\n");
        exit(0);
    }

    printf("Enter the numbers: \n");
    for (i = 0; i < n; i++)
        scanf("%d", &numbers[i]);    for (i = 0; i < n; i++) {
        if (numbers[i] < 0) {                       //Checking for negative numbers.
            for (k = i; k < n; k++) {               //If it find a negative number it deletes it by placing the forward element.
                numbers[k] = numbers[k + 1];        //into it's place and reduces the number of elements by 1.
            }
            n--;
            i--;
            continue;
        }

        if (numbers[i] == 0)
            continue;
        for (j = 0; j < numbers[i]; j++) {
            if (numbers[i] == fib) {
            f0 = 0;
            f1 = 1;
            fib = 1;
                break;
            }
            f0 = f1;
            f1 = fib;
            fib = f0 + f1;                      //It goes through all the fibonacci numbers to see if that element fits.
            if (numbers[i] < fib) {
                for (k = i; k < n; k++) {          //If we reach fibonacci that is bigger than that element does fit and we remove it.
                    numbers[k] = numbers[k + 1];
                }
                n--;
                i--;
                f0 = 0;
                f1 = 1;
                fib = 1;
                continue;
            }
        }
    }
    printf("This is the resulting array: \n");
    for (i = 0; i < n; i++)
        printf(" %d ", numbers[i]);

    exit(0);
}

 

[doktorwho]

But i have a different problem now. It still won't run in the VS. The results i got were in repl.it ide and this is what i get when i put it in VS:

1>------ Build started: Project: Assigments_for_PP2, Configuration: Debug Win32 ------
1>dd1.c
1>c:\users\doktorwho\desktop\projects\assigments_for_pp2\assigments_for_pp2\dd1.c(7): error C2057: expected constant expression
1>c:\users\doktorwho\desktop\projects\assigments_for_pp2\assigments_for_pp2\dd1.c(7): error C2466: cannot allocate an array of constant size 0
1>c:\users\doktorwho\desktop\projects\assigments_for_pp2\assigments_for_pp2\dd1.c(7): error C2133: 'numbers': unknown size
1>c:\users\doktorwho\desktop\projects\assigments_for_pp2\assigments_for_pp2\dd1.c(12): warning C4996: 'scanf': This function or variable may be unsafe. Consider using scanf_s instead. To disable deprecation, use _CRT_SECURE_NO_WARNINGS. See online help for details.
1>c:\program files (x86)\windows kits\10\include\10.0.14393.0\ucrt\stdio.h(1269): note: see declaration of 'scanf'
1>c:\users\doktorwho\desktop\projects\assigments_for_pp2\assigments_for_pp2\dd1.c(24): warning C4996: 'scanf': This function or variable may be unsafe. Consider using scanf_s instead. To disable deprecation, use _CRT_SECURE_NO_WARNINGS. See online help for details.
1>c:\program files (x86)\windows kits\10\include\10.0.14393.0\ucrt\stdio.h(1269): note: see declaration of 'scanf'
1>Done building project "Assigments_for_PP2.vcxproj" -- FAILED.
========== Build: 0 succeeded, 1 failed, 0 up-to-date, 0 skipped ==========[/code]

 

[haruspex]

But i have a different problem now. It still won't run in the VS. The results i got were in repl.it ide and this is what i get when i put it in VS:

1>------ Build started: Project: Assigments_for_PP2, Configuration: Debug Win32 ------
1>dd1.c
1>c:\users\doktorwho\desktop\projects\assigments_for_pp2\assigments_for_pp2\dd1.c(7): error C2057: expected constant expression
1>c:\users\doktorwho\desktop\projects\assigments_for_pp2\assigments_for_pp2\dd1.c(7): error C2466: cannot allocate an array of constant size 0
1>c:\users\doktorwho\desktop\projects\assigments_for_pp2\assigments_for_pp2\dd1.c(7): error C2133: 'numbers': unknown size
1>c:\users\doktorwho\desktop\projects\assigments_for_pp2\assigments_for_pp2\dd1.c(12): warning C4996: 'scanf': This function or variable may be unsafe. Consider using scanf_s instead. To disable deprecation, use _CRT_SECURE_NO_WARNINGS. See online help for details.
1>c:\program files (x86)\windows kits\10\include\10.0.14393.0\ucrt\stdio.h(1269): note: see declaration of 'scanf'
1>c:\users\doktorwho\desktop\projects\assigments_for_pp2\assigments_for_pp2\dd1.c(24): warning C4996: 'scanf': This function or variable may be unsafe. Consider using scanf_s instead. To disable deprecation, use _CRT_SECURE_NO_WARNINGS. See online help for details.
1>c:\program files (x86)\windows kits\10\include\10.0.14393.0\ucrt\stdio.h(1269): note: see declaration of 'scanf'
1>Done building project "Assigments_for_PP2.vcxproj" -- FAILED.
========== Build: 0 succeeded, 1 failed, 0 up-to-date, 0 skipped ==========[/code]

Not sure why it doesn't like it, but try moving the decalaration of MAX_NUM_EL in front of main().
You might care to replace the deprecated scanf as suggested above.

 

[haruspex]

I fixed the algortihm and it works good now. It fixed the issue of if numbers that are fibonacci come first the former are not displayed. The problem was that i didn't reset the fib number and it stayed at the value of the first fib.
Here it is:

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

//Program that from the numbers in the array searches for non-fibonacci numbers and then deletes them from the list.
int main(void) {
    static const int MAX_NUM_EL = 500;
    int numbers[MAX_NUM_EL];
    int i, n;
    int k, j, f0 = 0, f1 = 1, fib = 1;

    printf("Enter the size of your array: \n");
    scanf("%d", &n);
    if (n>MAX_NUM_EL || n<0) {
        printf("You've typed an invalid length of the array\n");
        exit(0);
    }
    if (n == 0) {
        printf("Your array has no elements to check. The program will exit now\n");
        exit(0);
    }

    printf("Enter the numbers: \n");
    for (i = 0; i < n; i++)
        scanf("%d", &numbers[i]);    for (i = 0; i < n; i++) {
        if (numbers[i] < 0) {                       //Checking for negative numbers.
            for (k = i; k < n; k++) {               //If it find a negative number it deletes it by placing the forward element.
                numbers[k] = numbers[k + 1];        //into it's place and reduces the number of elements by 1.
            }
            n--;
            i--;
            continue;
        }

        if (numbers[i] == 0)
            continue;
        for (j = 0; j < numbers[i]; j++) {
            if (numbers[i] == fib) {
            f0 = 0;
            f1 = 1;
            fib = 1;
                break;
            }
            f0 = f1;
            f1 = fib;
            fib = f0 + f1;                      //It goes through all the fibonacci numbers to see if that element fits.
            if (numbers[i] < fib) {
                for (k = i; k < n; k++) {          //If we reach fibonacci that is bigger than that element does fit and we remove it.
                    numbers[k] = numbers[k + 1];
                }
                n--;
                i--;
                f0 = 0;
                f1 = 1;
                fib = 1;
                continue;
            }
        }
    }
    printf("This is the resulting array: \n");
    for (i = 0; i < n; i++)
        printf(" %d ", numbers[i]);

    exit(0);
}

Looks ok. Just a few comments...

The word is "length", not "lenght".

Do you need the test for numbers<0? What would happen if you took it out?

You initialise f0, f1 and fib in three places. If you do it in the most logical place you only need do it once.

To avoid the repeated array shuffling, you could keep a separate read index and write index. If you read a Fib number then you write it back at the write index position in the array and increment both indexes. If you read a non-Fib number you only increment the read index.

 

[Dr Transport]

Not that I have time today to mess with it, I might next week, but I'd just write both of the arrays to a vector and compare vector arrays. If you find an element that isn't a Fibonacci number, it is easily eliminated by erase function.

 

[doktorwho]

Looks ok. Just a few comments...

The word is "length", not "lenght".

Do you need the test for numbers<0? What would happen if you took it out?

You initialise f0, f1 and fib in three places. If you do it in the most logical place you only need do it once.

To avoid the repeated array shuffling, you could keep a separate read index and write index. If you read a Fib number then you write it back at the write index position in the array and increment both indexes. If you read a non-Fib number you only increment the read index.

Not that I have time today to mess with it, I might next week, but I'd just write both of the arrays to a vector and compare vector arrays. If you find an element that isn't a Fibonacci number, it is easily eliminated by erase function.

I do not know how to do what you Dr Transport said and I'm trying to solve it as a beginner. I hope you can post your solution when you find time to do it :).
It is absolutely true what you haruspex said and i do not need to check the negative numbers as they will already be checked if i only make the small correction i made and that is to make the counter go to the absolute value of the element. You will see where i did it as i will remark on it in the code.
You will also see that i added two more function and that is repetition until a negative number is inputed for the array length :) and the array sorting at the beginning. I also placed the constant declaration above the main as you suggested.

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

    static const int MAX_NUM_EL = 500;
  
int main(void) {
  while(1) {
    int numbers[MAX_NUM_EL];
    int i, n, t;
    int k, j, f0 = 0, f1 = 1, fib = 1;
  
    printf("Enter the size of your array: \n");
    scanf("%d", &n);
    if (n>MAX_NUM_EL || n<0) {
        printf("You've typed an invalid length of the array\n");
        exit(0);
    }
    if (n == 0) {
        printf("Your array has no elements to check. The program will exit now\n");
        exit(0);
    }

    printf("Enter the numbers: \n");
    for (i = 0; i < n; i++)
        scanf("%d", &numbers[i]);
  
    for (i = 0; i < n; i++) {
      for (k = i+1; k < n; k++) {
        if (numbers[i] > numbers[j]) {
          t = numbers[i];
          numbers[i] = numbers[j];
          numbers[j] = t;
        }
      }
    }
  
    for (i = 0; i < n; i++) {
        if (numbers[i] == 0)
            continue;
        for (j = 0; j < abs(numbers[i]); j++) {        //The correction.
            if (numbers[i] == fib) {
                f0 = 0;
                f1 = 1;
                fib = 1;
                break;
            }
            f0 = f1;
            f1 = fib;
            fib = f0 + f1;                     
            if (numbers[i] < fib) {
                for (k = i; k < n; k++) {         
                    numbers[k] = numbers[k + 1];
                }
                n--;
                i--;
                f0 = 0;
                f1 = 1;
                fib = 1;
                continue;
            }
        }
    }
    printf("This is the resulting array: \n");
    for (i = 0; i < n; i++)
        printf(" %d ", numbers[i]);
      
    printf("\n");
  }
}

 

[doktorwho]

Not sure why it doesn't like it, but try moving the decalaration of MAX_NUM_EL in front of main().
You might care to replace the deprecated scanf as suggested above.

Im not sure what's wrong with the scanf function. There is still however this when i run it on VS:
1>------ Build started: Project: Assigments_for_PP2, Configuration: Debug Win32 ------
1>dd1.c
1>c:\users\doktorwho\desktop\projects\assigments_for_pp2\assigments_for_pp2\dd1.c(8): error C2057: expected constant expression
1>c:\users\doktorwho\desktop\projects\assigments_for_pp2\assigments_for_pp2\dd1.c(8): error C2466: cannot allocate an array of constant size 0
1>c:\users\doktorwho\desktop\projects\assigments_for_pp2\assigments_for_pp2\dd1.c(8): error C2133: 'numbers': unknown size
1>c:\users\doktorwho\desktop\projects\assigments_for_pp2\assigments_for_pp2\dd1.c(13): warning C4996: 'scanf': This function or variable may be unsafe. Consider using scanf_s instead. To disable deprecation, use _CRT_SECURE_NO_WARNINGS. See online help for details.
1>c:\program files (x86)\windows kits\10\include\10.0.14393.0\ucrt\stdio.h(1269): note: see declaration of 'scanf'
1>c:\users\doktorwho\desktop\projects\assigments_for_pp2\assigments_for_pp2\dd1.c(25): warning C4996: 'scanf': This function or variable may be unsafe. Consider using scanf_s instead. To disable deprecation, use _CRT_SECURE_NO_WARNINGS. See online help for details.
1>c:\program files (x86)\windows kits\10\include\10.0.14393.0\ucrt\stdio.h(1269): note: see declaration of 'scanf'
1>Done building project "Assigments_for_PP2.vcxproj" -- FAILED.
========== Build: 0 succeeded, 1 failed, 0 up-to-date, 0 skipped ==========

Is there anything that could help?

 

[Dr Transport]

I ran your code in VS2015 without making any changes what so ever...

 

[doktorwho]

I ran your code in VS2015 without making any changes what so ever...

I have VS2017. Maybe some of my settings are messed up, should i reset them?

 

[Dr Transport]

couldn't tell you, i just opened a new project witha precompiled header then copied your code in and it compiled without any errors.

 

[Mark44]

I'm running VS 2015, and I'm getting the same errors you got. VS doesn't like your declaration for MAX_NUM_EL, but I don't see why at the moment.

 

[Mark44]

You can't do this...

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

    static const int MAX_NUM_EL = 500; <<<<
 
int main(void) {
  while(1) {
    int numbers[MAX_NUM_EL];
   ...

The size of the array has to be known at compile time. MAX_NUM_EL is a variable, and the values of variables are known at run time, which always happens after compile time. That's the answer for C90, the specification from 1990. For compilers that support the C99 and C11 standards, you can do the above if the array could be a VLA (variable length array).

Here's an example from the MSDN documentation for the C2057 compiler error, the first one in your list of errors.

// C2057b.c
#define ArraySize1 10
int main() { 
   const int ArraySize2 = 10; 
   int h[ArraySize2];   // C2057 - C does not allow variables here
   int h[ArraySize1];   // OK - uses preprocessor constant
}

BTW, now that main() is declared as "int main(void)" you really should have a return statement down at the bottom.

 

[haruspex]

Some thoughts on optimisation, though not needed for this exercise I'm sure.

If you wanted it to perform well with very large numbers as inputs, there is an interesting way to avoid having to generate all the Fibonacci numbers up that number.

The n^th Fibonacci number is given by the formula $\frac 1{\sqrt 5}(\lambda^n-(1-\lambda)^n)$, where $\lambda=\frac{1+\sqrt 5}2$. Note that |1-λ|<1, so as n tends to infinity that term tends to zero. Indeed, it is very quickly the case that we can discard that term and just take the nearest whole number to what results. Consequently we can invert the formula to find whereabouts in the sequence a given Fibonacci x number belongs: its index is the nearest whole number to $\frac{\ln(x\sqrt 5)}{\ln(\lambda)}$.
Assuming the built-in exp and ln functions are rather faster than generating the Fibonacci sequence in a loop, we can calculate the n for a proposed Fibonacci number x, then reverse the process to find the actual n^th Fibonacci number and compare it with x.

 

[haruspex]

The size of the array has to be known at compile time.

Ah, of course! There may be other local scope variables, and the compiler has to know the offset of each from the current stack position.

An alternative solution would be to "new" the array.

 

[doktorwho]

Some thoughts on optimisation, though not needed for this exercise I'm sure.

If you wanted it to perform well with very large numbers as inputs, there is an interesting way to avoid having to generate all the Fibonacci numbers up that number.

The n^th Fibonacci number is given by the formula $\frac 1{\sqrt 5}(\lambda^n-(1-\lambda)^n)$, where $\lambda=\frac{1+\sqrt 5}2$. Note that |1-λ|<1, so as n tends to infinity that term tends to zero. Indeed, it is very quickly the case that we can discard that term and just take the nearest whole number to what results. Consequently we can invert the formula to find whereabouts in the sequence a given Fibonacci x number belongs: its index is the nearest whole number to $\frac{\ln(x\sqrt 5)}{\ln(\lambda)}$.
Assuming the built-in exp and ln functions are rather faster than generating the Fibonacci sequence in a loop, we can calculate the n for a proposed Fibonacci number x, then reverse the process to find the actual n^th Fibonacci number and compare it with x.

Note that in my posted code above i have a little mistake where i for sorting somehow used $j$ instead of $k$ but that doesn't change the way VS behaves.
I believe that as a part of the problem we are tested to see if we could generate the sequence as a check but i found that too and it's quite interesting. There is also a check that involves perfect squares.
If a number $n$ is a Fib number then one of the following must be a perfect square $(5n^2 \pm 4)$. So i was thinking that if a am to implement this as a check i would do this calculation take the root then round up to the nearest whole number and if it matches n it is a fib number? Is there any faster built in function?
Also I'm currently downloading VS 2015 and i will be checking into what Mark44 has said.
Do you happen to know why my code doesn't work when i declare my constant like:

#define MAX_NUM_EL 50

Rather than the one that i have used?

 

[haruspex]

take the root then round up to the nearest whole number

Don't you mean, take the root and see if it is a whole number?

if it matches n it is a fib number?

While every Fib number n has the property that either 5n²+4 or 5n²-4 is a whole number, the converse is not true, so you cannot use it to show the number is Fib. However, you could use it as a way to rule out some numbers quickly and avoid having to generate the Fib sequence for those.

my code doesn't work when i declare my constant like:

Is that with the test case you posted or with an input of 50 or so numbers?

 

[doktorwho]

Don't you mean, take the root and see if it is a whole number?

While every Fib number n has the property that either 5n²+4 or 5n²-4 is a whole number, the converse is not true, so you cannot use it to show the number is Fib. However, you could use it as a way to rule out some numbers quickly and avoid having to generate the Fib sequence for those.

Is that with the test case you posted or with an input of 50 or so numbers?

I was wrong and it does work with #define. Also after installing VS2015 it shows no problems. It runs and does everything like it's suppose to. Don't know what it was that made it go wrong with VS 2017

 

[SlowThinker]

I am still not quite sure what the loop in j is doing. I think this line can be removed, and the code in the loop only performed once.

        for (j = 0; j < abs(numbers[i]); j++) {        //The correction.

Also the loop that you are using to remove an element is used at several places and has a mistake.

            for (k = i; k < n; k++) {         
                    numbers[k] = numbers[k + 1];
            }
            n--;
            i--;

It reads element numbers[n], that is not in the array. While read-only access to a random place in memory should not cause much trouble, it should still be avoided. The loop should only go to k < n-1 .

 

[doktorwho]

I am still not quite sure what the loop in j is doing. I think this line can be removed, and the code in the loop only performed once.Also the loop that you are using to remove an element is used at several places and has a mistake.

It reads element numbers[n], that is not in the array. While read-only access to a random place in memory should not cause much trouble, it should still be avoided. The loop should only go to k < n-1 .

The #j# loop is necessary as it creates the fibbonaci sequency up to a certain number in the array. You are right about the second part however and i fixed it now :) Thanks!
Do you happen to know how to solve my next bug: When i scan for the numbers in the array if i input let's say 9 numbers and I've said I am going to have 8 numbers that last one is transferred as the array length input for the second loop. Could i somehow tell the scanf function to scan only n numbers and ignore the rest?

 

[SlowThinker]

The #j# loop is necessary as it creates the fibbonaci sequency up to a certain number in the array. You are right about the second part however and i fixed it now :) Thanks!
Do you happen to know how to solve my next bug: When i scan for the numbers in the array if i input let's say 9 numbers and I've said I am going to have 8 numbers that last one is transferred as the array length input for the second loop. Could i somehow tell the scanf function to scan only n numbers and ignore the rest?

I see, so the loop should be something like

  while ((fib==1) || //run the loop at least once to remove negatives
             (fib<=numbers[i]))

but your version will work too.

As for the scanf, I don't actually use scanf at all but you may try to do something like

  while (getchar()!=EOF) { }

just before you ask for array length. It's supposed to read and throw away any characters left in stdin.

 

[Mark44]

As for the scanf, I don't actually use scanf at all but you may try to do something like

  while (getchar()!=EOF) { }

just before you ask for array length. It's supposed to read and throw away any characters left in stdin.

Or you could use fflush() to flush the input buffer.

fflush(stdin);

This is especially useful if you read in a number and then later read in a string of characters or even one character. Flushing the input buffer after reading a number prevents a following read from taking in the newline character.

Do you happen to know how to solve my next bug: When i scan for the numbers in the array if i input let's say 9 numbers and I've said I am going to have 8 numbers that last one is transferred as the array length input for the second loop. Could i somehow tell the scanf function to scan only n numbers and ignore the rest?

I'm not sure I understand your question. Are you are using scanf() to input more than one number at a time? It's easiest on the user of your program (which is not necessarily you) to input a single number per call to scanf().

You could issue a prompt that asks how many numbers are to be entered. Then use that number to control a for loop. Something like this:

printf("How many numbers do you want to enter?")
scanf("%d", &count);
for (i = 0; I < count; ++i)
{
   printf("> ");
   scanf("%d", &arr[ i]);
}

 

[doktorwho]

Or you could use fflush() to flush the input buffer.

fflush(stdin);

This is especially useful if you read in a number and then later read in a string of characters or even one character. Flushing the input buffer after reading a number prevents a following read from taking in the newline character.
I'm not sure I understand your question. Are you are using scanf() to input more than one number at a time? It's easiest on the user of your program (which is not necessarily you) to input a single number per call to scanf().

You could issue a prompt that asks how many numbers are to be entered. Then use that number to control a for loop. Something like this:

printf("How many numbers do you want to enter?")
scanf("%d", &count);
for (i = 0; I < count; ++i)
{
   printf("> ");
   scanf("%d", &arr[ i]);
}

I find it faster if the user inputs all the numbers in one row, as an easier way for the user to use the program but that has a bad side that i will pick up in my scanf more numbers if inputted. One at the time works like a charm and i really like the way you used > and I've implemented in in my code :)

 

[Mark44]

I find it faster if the user inputs all the numbers in one row, as an easier way for the user to use the program but that has a bad side that i will pick up in my scanf more numbers if inputted. One at the time works like a charm and i really like the way you used > and I've implemented in in my code :)

If you want it to be really fast, just initialize your matrix with the numbers you want and don't bother with scanf().

As far as inputting multiple numbers at the same time, you might find it faster, but if you write "real" software that others use, it can be very confusing to those people. And as you found out, things can go wrong even for the person who wrote the program. There are some real intricacies in the control string for scanf() with multiple inputs.

 

[newjerseyrunner]

I just want to let you know that if you just hit "run" on that code in a mingw ide and VS, they will act differently.

Mingw ides never launch the executable directly, they launch a launcher, which stays open until you close it. It allows you to see the results of any stdout before closing. VS doesn't do that, so you should place a pause (just wait for a cin) if you want to see the output in the ide.

 

[ChrisVer]

also you could try using the fibonacci number generator in a separate function- that way nobody would be confused with that j loop...
eg have a function: bool isFibonacci(int n), which can tell you if the integer number n is a Fibonacci number...?
That way you in a later time or other people looking through your code, wouldn't have to read the whole program and keep track of nested for loops in your code... afterall that for loop which tests your fibonnaci numbers can be separated from your main program. This will remove several lines from your main code, and you won't have to keep track of them when you return to reading your code a month later (as long as you tested them and know they work properly).
Also I am wondering how optimal generating all the fibonacci numbers is:
so for example it would be much better to have an array with fibonacci numbers up to the one corresponding to your max(numbers). So for example if you tested your fibonacci-ness of an entry 50, and saw that 1,2,3,5,8 etc were not equal to 50, why would you retry them for an entry 51 (>50) ? with arrays you can come up with several ways that can solve you this problem... eg:
1. have the fibonacci array of length M (taken from the maximum of your array with length N)...
2. sort the fibonacci array (time~M), and your array (time~N). Time's given if you use a counting sort algorithm.
3. start looking in your array elements, if your 0-th element is a fibonacci number and it is in the 5th position of the fibonacci array, you won't ever have to recheck the first 4 elements of the fibonacci array.
Of course there may be more intelligent ways to decrease your calculation time, but that's the 1st one that came to my mind...

 

[Buffu]

int MAX_NUM_EL = 500;
int numbers[MAX_NUM_EL];

If you are using C++ then what is the need of this line.
C++ have vectors. the header file is #include<vector>.

 

[ChrisVer]

If you are using C++ then what is the need of this line.

as far as I understood, he said he uses C.

 

[doktorwho]

also you could try using the fibonacci number generator in a separate function- that way nobody would be confused with that j loop...
eg have a function: bool isFibonacci(int n), which can tell you if the integer number n is a Fibonacci number...?
That way you in a later time or other people looking through your code, wouldn't have to read the whole program and keep track of nested for loops in your code... afterall that for loop which tests your fibonnaci numbers can be separated from your main program. This will remove several lines from your main code, and you won't have to keep track of them when you return to reading your code a month later (as long as you tested them and know they work properly).
Also I am wondering how optimal generating all the fibonacci numbers is:
so for example it would be much better to have an array with fibonacci numbers up to the one corresponding to your max(numbers). So for example if you tested your fibonacci-ness of an entry 50, and saw that 1,2,3,5,8 etc were not equal to 50, why would you retry them for an entry 51 (>50) ? with arrays you can come up with several ways that can solve you this problem... eg:
1. have the fibonacci array of length M (taken from the maximum of your array with length N)...
2. sort the fibonacci array (time~M), and your array (time~N). Time's given if you use a counting sort algorithm.
3. start looking in your array elements, if your 0-th element is a fibonacci number and it is in the 5th position of the fibonacci array, you won't ever have to recheck the first 4 elements of the fibonacci array.
Of course there may be more intelligent ways to decrease your calculation time, but that's the 1st one that came to my mind...

You make an interesting point, about the separate function and the fibbonacci array as well. And also for you number 3 prepositions, it's a great idea! I will try to come up with something and ill post to make a comparison as I'm deeply interested in the ways you suggested

 

[SlowThinker]

Well Fibonacci numbers grow quite fast, so the speedup probably won't be all that awesome. And the code will be a LOT longer, with many opportunities for an error.
If you want to make the check faster, I'd try to work with Haruspex's idea from post #23. This might work but I haven't tested it:

bool isFibonacci(int n)
 {
  static const double sqrt5=sqrt(5);
  static const double lnlam=1/ln((1+sqrt(5))/2);
  double error=ln(n*sqrt5)*lnlam;
  error=error-round(error);
  return fabs(error*n)<0.6; //magic constant, needs to be between 0.4 and 0.7
 }

It should only work for big n (I did a quick check with Excel and it seems to work for all numbers though, bigger than 0 of course).

 

[ChrisVer]

yes, that would make it way more faster (as fast as looping through your given array)...
Is there a reason for using the static keyword? Oh it's for doing the assignment only once?

 

[doktorwho]

Well Fibonacci numbers grow quite fast, so the speedup probably won't be all that awesome. And the code will be a LOT longer, with many opportunities for an error.
If you want to make the check faster, I'd try to work with Haruspex's idea from post #23. This might work but I haven't tested it:

bool isFibonacci(int n)
 {
  static const double sqrt5=sqrt(5);
  static const double lnlam=1/ln((1+sqrt(5))/2);
  double error=ln(n*sqrt5)*lnlam;
  error=error-round(error);
  return fabs(error*n)<0.6; //magic constant, needs to be between 0.4 and 0.7
 }

It should only work for big n (I did a quick check with Excel and it seems to work for all numbers though, bigger than 0 of course).

yes, that would make it way more faster (as fast as looping through your given array)...
Is there a reason for using the static keyword? Oh it's for doing the assignment only once?

I believe that as a part of the assignment I'm to show that i know how to generate a sequence. So its down to the suggestion made by ChrisVer and my own solution. Others although correct and faster have to have some sort of fibboncci sequence generation

 

[haruspex]

1. have the fibonacci array of length M (taken from the maximum of your array with length N)...

Having created the array of Fibonacci numbers up to and including the largest number read in, you can look each given number up very quickly using a binary chop.

 

[ChrisVer]

Having created the array of Fibonacci numbers up to and including the largest number read in, you can look each given number up very quickly using a binary chop.

hmm is the binary search faster than what I mentioned? I haven't thought about it... but I guess if you chop the fibonacci array as you go to larger numbers, it has to be (since mine was still linear).

 

[haruspex]

is the binary search faster than what I mentioned?

I believe so. And you do not need to sort the input, nor restore the allowed numbers to original order.

In fact, you can do a bit better than binary chop because the Fib sequence grows exponentially. To make the first guess, count the number of bits, B, in the test number and try index B*K where K=ln(2)/ln((1+√5)/2).

 

[rcgldr]

To get rid of warnings related to functions like scanf(), add this line before any includes in your source file:

#define _CRT_SECURE_NO_WARNINGS 1

VS doesn't support variable length arrays. If you want a stack based equivalent, use _alloca():

    int *numbers = _alloca(MAX_NUM_EL * sizeof(int));    /* include <malloc.h> */

For a 32 bit signed integer, the maximum Fibonacci number is fib(46) = 1836311903, so it will take 47 iterations to check for all possible Fibonacci numbers from fib(0) to fib(46). If using 64 bit unsigned integers, the max is fib(93) = 12200160415121876738, so it would take 94 iterations (fib(0) to fib(93)). For this program, the code could create a table for all Fibonacci numbers fib(0) to fib(46) and as suggested, do a binary search of the table.

 

[willem2]

For a 32 bit signed integer, the maximum Fibonacci number is fib(46) = 1836311903, so it will take 47 iterations to check for all possible Fibonacci numbers from fib(0) to fib(46). If using 64 bit unsigned integers, the max is fib(93) = 12200160415121876738, so it would take 94 iterations (fib(0) to fib(93)). For this program, the code could create a table for all Fibonacci numbers fib(0) to fib(46) and as suggested, do a binary search of the table.

The problem with the binary search are mispredicted jumps. The logarithm idea is better, but the a full precision log takes too long. You can do with a very unaccurate logarithm: just count the zero bits at the start of the number. since fibonaccy numbers are approcimately a factor of 1.618 apart, there will never be more than 2 Fibonacci numbers with the first 1 bit in the same place.

//smallest and largest Fibonacci numbers with a certain number of leading zero's
int table_smallest[] = { 1, 2, 5, 8, 21, 34, 89, 144, etc.
int table_largest[] =   { 1, 3, 5,13,21, 55, 89. 233, etc

bool is_fibo(int n)
{
    int bit;
    bit = _bit_scan_forward(n)
    return ((n == table1[bit]) | (n == table2[bit]);
}

If you can't get at a fast bit scan instruction, converting an int to a double and extracting the exponent may work.

 

[Mark44]

To get rid of warnings related to functions like scanf(), add this line before any includes in your source file:

#define _CRT_SECURE_NO_WARNINGS 1

Or you can do this up at the top of your code:

#pragma waring(disable: 4996)

scanf() and several other string functions have been deprecated in VS since about VS2012 or so, due to their connection with buffer overruns. The preferred input routine to use in place of scanf() is scanf_s(), which is specific to Visual Studio.

 

[Dr Transport]

...
VS doesn't support variable length arrays. If you want a stack based equivalent, use _alloca():

That is the beauty of vectors, you don't have to allocate them in the beginning. Read the numbers into one, if they are out of order, so what, do a sort, you can check it for negatives and eliminate them easily. To check if any are Fibonacci numbers, generate a vector of them and check if the Fibonacci number is larger than the max of the array, then you can stop that generation. Do a binary search and your done.