Thread: Please help me optimize my code

  1. #1
    Apprentice
    Join Date
    Oct 2006
    Location
    LA
    Posts
    53

    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();
    }

  2. #2
    Registered User
    Join Date
    Jan 2005
    Posts
    7,366
    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.

  3. #3
    Registered User
    Join Date
    Mar 2009
    Posts
    48
    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;
    }
    Last edited by zalezog; 01-25-2010 at 03:05 AM.

  4. #4
    The larch
    Join Date
    May 2006
    Posts
    3,573
    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.
    I might be wrong.

    Thank you, anon. You sure know how to recognize different types of trees from quite a long way away.
    Quoted more than 1000 times (I hope).

Popular pages Recent additions subscribe to a feed

Similar Threads

  1. Replies: 6
    Last Post: 12-19-2007, 12:24 PM
  2. Proposal: Code colouring
    By Perspective in forum A Brief History of Cprogramming.com
    Replies: 28
    Last Post: 05-14-2007, 07:23 AM
  3. Values changing without reason?
    By subtled in forum C Programming
    Replies: 2
    Last Post: 04-19-2007, 10:20 AM
  4. Obfuscated Code Contest
    By Stack Overflow in forum Contests Board
    Replies: 51
    Last Post: 01-21-2005, 04:17 PM
  5. Interface Question
    By smog890 in forum C Programming
    Replies: 11
    Last Post: 06-03-2002, 05:06 PM