Please help me optimize my code

I have written a simple code to practice some basics of c++ and some file handling. Haven't really used any of the OOP features. The program should open the input file and read each individual number from the input file. For each number “n” read from the input file, the program should write out the first “n” Fibonacci numbers to the output file. Please help me find bugs and improve my code.

Code:

---

#include "fibonacci.h"
using namespace std;

int main()
{
        int number;

        ifstream input;
        ofstream output;

        string inputfile;
        string outputfile;

        vector<int> validNumbers;
        vector<int> invalidNumbers;

        cout<<endl<<endl;

        cout<<"Enter the input file name : ";
        cin>>inputfile;

        input.open(inputfile.c_str());
        if(input.fail())
                cout<<endl<<"Could not open input file"<<endl;
        else
        {
                cout<<"Enter the output file name : ";
                cin>>outputfile;

                cout<<"Processing the input file..."<<endl;
                while(!input.eof())
                {
                        input>>number;
                        if( number<=0 )
                                cout<<endl<<"Skipping "<<number<<" as no fibonacci series is possible"<<endl;
                        else
                                ComputeFibonacci( outputfile.c_str(), number );
                }

                input.close();
        }
}

---

Code:

---

#include "fibonacci.h"
using namespace std;

void ComputeFibonacci(const char* fileName, int number)
{
        int first = 0;
        int second = 1;
        ofstream output;

        output.open(fileName, ios_base::app);
        if(output.fail())
                cout<<endl<<"Can not open the output file"<<endl;
        else
                output<<endl<<endl<<"Number read : "<<number<<endl;

        output<<"The fibonacci series is : "<<first<<"\t"<<second;

        for(int i=2;i<number;i++)
        {
                int temp = second;
                second = first + second;
                first = temp;
                output<<"\t"<<second;
        }

        output.close();
}

---

A couple quick ones:

1) Don't use while(!input.eof()) in that way to read in a loop. Instead, call the reading function inside the while control:

Code:

---

while (input>>number)

---

One reason is that if the input is bad (for example, a non-numeric character in your input file), your loop will continue indefinitely. Another reason is that sometimes using eof() to control the loop leads the loop to run one too many times.

2) Don't declare your variables at the top of each function, declare them when and where you first need them.

If the input file contains numbers which are repeated, then calculating the same sequence is an overhead, why not store those numbers in a vector or an array.

For a particular number whose fibonacci sequence is to be displayed, you walk through the array or vector and display its contents , if you find that the requested number is greater than numbers in the array or vector then you find the fibonacci sequence upto the requested number and simultaneously store the sequence.

There won't be any noticeable difference for small dataset but for larger datasets , this might help.

Edit :
Something like this:

Code:

---

#include<iostream>
#include<vector>

int main(void) {
        std::vector<unsigned long> sequence;
    unsigned long number;
   
        //Minimum 2 elements in the vector
        sequence.push_back(0); //First element
        sequence.push_back(1);//Second element
   
        unsigned long i;

        while (std::cin >> number) {
                for (i = 0; i < sequence.size() && i < number; i++)
                        std::cout << sequence[i] << " ";
       
                unsigned long first = sequence[sequence.size() - 2];
                unsigned long  second = sequence[sequence.size() - 1];
               
                for (; i < number; i++) {
                    unsigned long temp = second;
                    second = first + second;
                    first = temp;
                    sequence.push_back(second);
                    std::cout << second << " ";
                }
                std::cout << std::endl;
        }
        return 0;
}

---

Quote:

---

There won't be any noticeable difference for small dataset but for larger datasets , this might help.

---

Fibonacci numbers grow so fast that (about) the 48th value overflows 32-bit long unsigned, and the 94th overflows a 64-bit unsigned. There isn't much hope to calculate longer sequences without a big number library.