Fibonacci series

Question

I have written the following code for my 1st year project, the Fibonacci series. I programmed it without searching Google. Is my code good enough to get me an A or a B? Can I make the code more professional?

#include <stdio.h>
#include <windows.h>
#include <math.h>
main(){
 double x, y, xy;
 printf("Presenting to you the Fibonacci Numbers");
 Sleep(2500);
 x = 0;
 y = 1;
 printf("\n%.0f", x);
 printf("\n%.0f", y);
 while (1){
 xy = x + y;
 y = y + xy;
 x = xy;
 printf("\n%.0lf", x);
 Sleep(250);
 printf("\n%.0lf", y);
 Sleep(250);
 }}

Answer 1

1) The Fibonacci series shall never yield non whole numbers; doubles are unnecessary and possibly computationally expensive. Try a uintmax_t. It will probably give you values of up to 2^64 - 1 (18,446,744,073,709,551,615). That should certainly be sufficient. Additionally, I would put in code to terminate the program as it reaches the upper limit of uintmax_t.

2) Your could have a classic I/O problem: output buffering. Try to put a \n in every print statement to encourage proper flushing. So rather than having the come at the beginning of each print statement, try putting it at the end. If you like it the way it is, you can call fflush(stdout) after every printf(). While we're on the topic, your first two printf() statements can be combined. More info at this SO question.

3) Obviously the Sleep() functions are unnecessary to your code. I presume that they are there for the aesthetic value of the output, but I would take them out. Doing so would make your code OS-independent as well as neater. But obviously, depending on the assignment, appearance may be above all.

4) Finally, despite the mathematical purpose of the program, math.h is not needed.

Addendum: You asked how to make it more professional. You could make a Fibonacci function encapsulating the algorithmic properties of your code, while leaving the aesthetic properties (sleep, welcome message) in main(). Example:

uintmax_t num = get_next_fibbonaci(/*reference to struct storing state || values necessary for calculation*/);

• Roland Illig: Global variables would not be considered professional in this situation.
• Peter Cordes: Using static variables inside a generator function would be less bad than globals, but is not a good pattern for reusable code: it's not thread-safe. A more modern design could have a state object that the caller passes by reference.
• user692992: @Peter Cordes Given that this is a single-thread program that's a one-off for this singular goal, I believe persistent variables would be an acceptable solution. However, I can see how it might not be the most "professional" way. Edited answer
• Peter Cordes: Yup, good edit. But note that you can't just pass the two history values by value because this function has to update them, or else the caller has to know about Fibonacci logic. So || values necessary for calculation isn't really a clear logical or.
• user692992: I was thinking x3 = fib(x1,x2)

Answer 2

Is my code good enough to get me an A or a B?

Produces incorrect output

After a while the code produces wrong output. I would not mind so much that code has a limited range, but one that starts producing errors, without warning, is not good. B for effort - I admire your goals, C for implementation.

FP for an integer problem

The Fibonacci series runs though various odd numbers, resulting in incorrect results after the precision of double is used up at about 2^DBL_MANT_DIG or typically 2⁵³. unsigned long long affords at least 2⁶⁴-1.

Although using double allows for a greater range, I'd see using floating point (FP) as a weaker approach to this problem than integers and strings.

Unneeded code

Sleep(2500); no functional purpose to the calculation of the Fibonacci series. Use of magic numbers 2500, 250 lacks explanation.

main() lacks return type

This style of programming went out 20 years ago. Code the return type.

// main(){
int main() {
// or clearly
int main(void) {

Consider declaring object when/where needed

Example

//double x, y, xy;
//...
//while (1){
// xy = x + y;
double x = 0;
double y = 1;
...
while (1) {
 double xy = x + y;

Strange to use end-of-line to start printing

A more common idiom is '\n' afterwards and facilitates flushing stdout when line buffered.

//printf("\n%.0lf", y);
printf("%.0lf\n", y);

Minor: odd formatting

}} is uncommon. Perhaps:

 }
}

---

Can I make the code more professional?

Stop before the algorithm can no longer provide correct output.

• Lundin: In addition, main() with a missing return 0; invokes K&R style undefined behavior. You can only leave out return 0 from C99 and beyond, but then you can't use main() any longer.
• chux: @Lundin Interesting. For me, I prefer int main(void) { .... return 0;}.
• Lundin: Specifically the standard says that if the caller uses the return value from a function, omitting it results in undefined behavior. And since UNIX OS has been using the return value of programs since the dawn of time, programs such as K&R main() { printf("hello, world\n"); } invokes undefined behavior. Perhaps this one is the first C bug ever written, even.
• Lundin: And int main (void) is correct, because int main() is obsolete style from C99 and beyond. Still marked as obsolete in C17 though, so I guess they won't remove it.
• UnfreeHeX: @ReinstateMonica Yea, i figured out a way to avoid the problem that is caused by the series, if it runs for a long time. i used %.3g instead of %lf so its kinda working fine now.

Answer 3

That should certainly be sufficient

2^64 is a lot, but mathematics is stronger. Instead of just choosing a big number, you should stop before it overflows.

#include <stdio.h>
void main(void) {
 unsigned long MAX = -1;
 unsigned long a = 0, b = 1, new;
 do {
 new = a + b;
 printf("%lu\n", new);
 a = b;
 b = new;
 } while (new < MAX/2);
}

There were quite a few problems before I got it right. The post-test loop helps, as does a correct %ul in printf().

---

Your two-in-one algorithm looks efficient, but I don't fully understand. I guess you save a variable swapping per iteration. I would give you an A for that. The rest has already been discussed.