HiveBrain v1.2.0
Get Started
← Back to all entries
patterncppMinor

Outputting Fibonacci numbers

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
outputtingnumbersfibonacci

Problem

My program gets some input values and outputs Fibonacci values.

Generally, I'm adding single numbers from the right to left, and I'm saving it in the sum value and get rest of it and add to string. (The value of "second" will always be larger than "first").

Is there some way to speed up my code?

#include 
#include 
using namespace std;

string addition(string &first, string &second);

int main()
{
      unsigned int value;
      cin>>value;
      string first="1";                 
      string second="1";                
      string helper;                         

      if(value==1)
          cout=0;i--)
      {
             if(minimum)
             {
                  minimum--;
                  sum=first[minimum]-48+second[i]-48+moveValue;    
                  moveValue=sum/10;                             
                  app=sum%10+48;
                  finalValue=app+finalValue;
             }
             else
             {
                  sum=second[i]-48+moveValue;
                  moveValue=sum/10;
                  app=sum%10+48;
                  finalValue=app+finalValue;
             }
      }

      if(moveValue==1)
      {
           app=moveValue+48;
           finalValue=app+finalValue;
      }
      return finalValue;
}

Solution

I see a number of things that you could use to improve your code.

Don't abuse using namespace std

Putting using namespace std at the top of every program is a bad habit that you'd do well to avoid. Know when to use it and when not to (as when writing include headers).

Avoid introducing unneeded variables

The helper string is not necessary. Instead, you could rework your addition routine so that it calculates first+=second and then swaps first and second. In that way, second would always contain the final value. This would also help with the next suggestion.

Avoid introducing unneeded tests

With a little judicious rewrite, your main function can eliminate the special case testing to see if the terms value is 1 or 2. I'd rewrite it like this:

int main()
{
    unsigned int maxterm;
    std::cin >> maxterm;
    std::string first="1";                 
    std::string second="1";                

    for(unsigned i=2; i < maxterm; ++i) {
        fibnext(first,second);
    }
    std::cout << second << std::endl;
}


Note that I've also renamed a variable from value (which is not very descriptive) to maxterm and the routine from addition (which is descriptive but very general) to fibnext which more accurately suggests that it's calculating the next Fibonacci term.

Simplify your addition

There are a few ways to simplify the addition, but this is perhaps the most straightforward:

std::string &fibnext(std::string &first, std::string &second)
{
    char carry = 0;

    // make strings the same length by 0-padding first
    first.insert(0, second.length() - first.length(), '0');
    for(int i=first.length()-1; i >= 0; --i)
    {
        first[i] += second[i] - '0' + carry;
        if (first[i] > '9') {
            first[i] -= 10;
            carry = 1;
        } else {
            carry = 0;
        }
    }
    if (carry)
        first.insert(0, 1, '1');

    std::swap(first,second);
    return second;
}


Results

The effects of these changes are to reduce the number of times that strings are copied and constructed. In fact, only two std::string objects are created and then expanded as needed. Each time a digit is inserted at the beginning of a string, it (conceptually, anyway) needs to allocate a new space that's large enough, copy the string into that new memory, and then free the old space. However, std::string implementations are often smart enough to allocate larger-than-immediately-needed chunks to help with exactly this type of operation.

I measure it as being almost 30x faster than the original code (0.10 seconds vs 2.88 seconds on my 64-bit Linux box) when calculating the 10,000th Fibonacci number.

Code Snippets

int main()
{
    unsigned int maxterm;
    std::cin >> maxterm;
    std::string first="1";                 
    std::string second="1";                

    for(unsigned i=2; i < maxterm; ++i) {
        fibnext(first,second);
    }
    std::cout << second << std::endl;
}
std::string &fibnext(std::string &first, std::string &second)
{
    char carry = 0;

    // make strings the same length by 0-padding first
    first.insert(0, second.length() - first.length(), '0');
    for(int i=first.length()-1; i >= 0; --i)
    {
        first[i] += second[i] - '0' + carry;
        if (first[i] > '9') {
            first[i] -= 10;
            carry = 1;
        } else {
            carry = 0;
        }
    }
    if (carry)
        first.insert(0, 1, '1');

    std::swap(first,second);
    return second;
}

Context

StackExchange Code Review Q#107121, answer score: 5

Revisions (0)

No revisions yet.