Basic Fibonacci sequence in C

Things you could improve

Using Headers

-
Right now you are defining some items at the top of your source file.

#define RED     "\x1b[31m"
#define GREEN   "\x1b[32m"
#define YELLOW  "\x1b[33m"
#define BLUE    "\x1b[34m"
#define MAGENTA "\x1b[35m"
#define CYAN    "\x1b[36m"
#define RESET   "\x1b[0m"

These are all relatable, and reusable ANSI escape codes to color your text in a terminal, so you could put them all in their own header (color.h) for easy use with other files.

#define BLACKTEXT(x) "\033[30;1m" x "\033[0m"
#define REDTEXT(x) "\033[31;1m" x "\033[0m"
#define GREENTEXT(x) "\033[32;1m" x "\033[0m"
#define YELLOWTEXT(x) "\033[33;1m" x "\033[0m"
#define BLUETEXT(x) "\033[34;1m" x "\033[0m"
#define MAGENTATEXT(x) "\033[35;1m" x "\033[0m"
#define CYANTEXT(x) "\033[36;1m" x "\033[0m"
#define WHITETEXT(x) "\033[37;1m" x "\033[0m"

Then you can just #include "color.h" in all of your files where you need some color in your terminal.

-
As you can see from my own color.h header file, I don't have a RESET macro. I didn't find a use for it, since I would always end up resetting the color right after I used it. The way I have it defined, my macros can do that for me.

User Experience

-
I find it useless to have to enter a first and second number.

printf("Enter first number > ");
  scanf("%d",&n1);
        printf("Enter the seconed number > ");
  scanf("%d",&n2);

What is the point of this? Just start from \$ 0 \$ and \$ 1 \$, and go to \$ n \$ number of terms.

Efficiency/Algorithm

-
Right now you can't compute that many fibonacci numbers, because of the maximum size of an int; you can only calculate numbers up to 32767.

We aren't expecting negative numbers in our sequence, so we easily increase this restriction, you could use an unsigned int which has a maximum value of 65535, or even better, a long long unsigned int with a maximum value of 18446744073709551615. That's 562967131565294 times larger than your current program!

-
Your algorithm can be simplified down using method extraction and recursion.

for (i; i =1) { pos ++; printf( RESET "%i\n" RESET, n2); }
                if (n2 <=0) { neg ++; printf( MAGENTA   "%i\n" MAGENTA,  n2); }
    int temp = n2;

    n2 += n1;
    n1 = temp;
  }

Move the computing of the fibonacci sequence to it's own method. All you should be passing it is the number of the term you want to compute.

unsigned long long int fibonacci(unsigned long long int n)
{
    if (memoization[n] != -1) return memoization[n];

    return (n < 2)? n : (memoization[n] = fibonacci(n-1) + fibonacci(n-2));
}

-
You might notice that I have included an array in my previous point named memoization. We want to use this to decrease computation time with the recursion. Basically, if we have already computed a number, we put it into the array so we don't have to calculate it the next time we want to find a fibonacci number. If we haven't found the number, we can indicate that in the array by storing a -1 there instead.

Style

-
Put all of your variable declarations to separate lines. From Code Complete, 2nd Edition, p. 759:


With statements on their own lines, the code reads from top to bottom,
instead of top to bottom and left to right. When you’re looking for a
specific line of code, your eye should be able to follow the left
margin of the code. It shouldn’t have to dip into each and every line
just because a single line might contain two statements.

-
I can't follow your tabbing scheme.

printf("Enter first number > ");
  scanf("%d",&n1);
        printf("Enter the seconed number > ");
  scanf("%d",&n2);
        printf("Enter the number of terms > ");
  scanf("%d",&limit);

This makes it harder for me to read. You should align all of the left characters in a block to make them more readable.

-
You should declare your parameters as void if you don't take in any arguments.

int main(void)

Standards

-
You should be declaring i inside of your for loops.(C99)

for (int i = 0; i < limit; i++)

Final code:

• Iterative: \$ O(1) \$

The calculation of the fibonacci sequence here is a bit different than my point I made above. I didn't mention it, since it was so different than how you were calculating your sequence. This is one of the most optimal solutions for calculating a fibonacci sequence.

#include 
#include 
#include 

long long unsigned int fibonacci(long long unsigned int n)
{
    return round((1/sqrt(5)) * (pow(((1 + sqrt(5)) / 2), n) - pow(((1 - sqrt(5)) / 2), n)));
}

int main(void)
{
    unsigned int num = 0;

    printf("Enter how far to calculate the series: ");
    if (scanf("%i", &num) <= 0)
    {
        puts("Invalid input.");
        return -1;
    }

    for(unsigned int n = 1; n < num + 1; ++n) printf("%llu\n", fibonacci(n));
}

• Recursion: \$ O(n) \$

```
#include
#include

#define ARRAYLENGTH 100 // keep in mind the result must fit in an llu in

StackExchange Code Review Q#46880, answer score: 11