Dismiss Notice
Join Physics Forums Today!
The friendliest, high quality science and math community on the planet! Everyone who loves science is here!

C/++/# Reviewing my C code

  1. Mar 31, 2017 #1
    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?
    Code (C):

    #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 lenght 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;
    }
     
     
  2. jcsd
  3. Mar 31, 2017 #2

    ChrisVer

    User Avatar
    Gold Member

    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?
     
  4. Mar 31, 2017 #3
    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:
    Code (C):

    #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 lenght 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 im gonna do some simplifications if possible. Hmm i don't follow your n-- argument. Could you explain a little more?
     
  5. Mar 31, 2017 #4

    ChrisVer

    User Avatar
    Gold Member

    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).
     
  6. Mar 31, 2017 #5

    haruspex

    User Avatar
    Science Advisor
    Homework Helper
    Gold Member
    2016 Award

    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?
     
  7. Mar 31, 2017 #6

    haruspex

    User Avatar
    Science Advisor
    Homework Helper
    Gold Member
    2016 Award

    Sounds like you need a flush call to push out the last of the print queue.
     
  8. Mar 31, 2017 #7
    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.
     
    Last edited: Mar 31, 2017
  9. Mar 31, 2017 #8
    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.
     
    Last edited: Mar 31, 2017
  10. Mar 31, 2017 #9

    Mark44

    Staff: Mentor

    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.
     
  11. Apr 1, 2017 #10
    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?
     
  12. Apr 1, 2017 #11
    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:
    Code (C):

    #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 lenght 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);
    }
     
     
  13. Apr 1, 2017 #12
    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]
     
  14. Apr 1, 2017 #13

    haruspex

    User Avatar
    Science Advisor
    Homework Helper
    Gold Member
    2016 Award

    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.
     
    Last edited: Apr 1, 2017
  15. Apr 1, 2017 #14

    haruspex

    User Avatar
    Science Advisor
    Homework Helper
    Gold Member
    2016 Award

    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.
     
    Last edited: Apr 1, 2017
  16. Apr 1, 2017 #15

    Dr Transport

    User Avatar
    Science Advisor
    Gold Member

    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.
     
  17. Apr 1, 2017 #16
    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.
    Code (C):

    #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");
      }
    }
     
     
    Last edited: Apr 1, 2017
  18. Apr 1, 2017 #17
    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?
     
  19. Apr 1, 2017 #18

    Dr Transport

    User Avatar
    Science Advisor
    Gold Member

    I ran your code in VS2015 without making any changes what so ever....
     
  20. Apr 1, 2017 #19
    I have VS2017. Maybe some of my settings are messed up, should i reset them?
     
  21. Apr 1, 2017 #20

    Dr Transport

    User Avatar
    Science Advisor
    Gold Member

    couldn't tell you, i just opened a new project witha precompiled header then copied your code in and it compiled without any errors.
     
Know someone interested in this topic? Share this thread via Reddit, Google+, Twitter, or Facebook

Have something to add?
Draft saved Draft deleted



Similar Discussions: Reviewing my C code
  1. Critique my C++ code (Replies: 11)

Loading...