patterncppMinor
Outputting Fibonacci numbers
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?
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
Putting
Avoid introducing unneeded variables
The
Avoid introducing unneeded tests
With a little judicious rewrite, your
Note that I've also renamed a variable from
Simplify your addition
There are a few ways to simplify the addition, but this is perhaps the most straightforward:
Results
The effects of these changes are to reduce the number of times that strings are copied and constructed. In fact, only two
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.
Don't abuse
using namespace stdPutting
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.