Please help C++ program!!!

[user4678]

I'm trying to create a program that computes the values of cos using the Taylor Series
as long as the absolute value of a term is greater than 0.0001 and prints the
number of terms used in the series approximation. My code is the following and as of right now I am getting the wrong result. Please help me fix it! I've spent hours trying to figure it out and don't know what to do.

Enter the angle in radians: 3.14
There were 8 numbers of terms used in the series approximation.
The calculated cosine of 3.14 is 0.00499852
The C++ cosine of 3.14 is -0.999999

// Pre-processor Directives
#include<iostream> // Required for for use of cin, cout
#include<cmath> // Required for use of sin, cos, pow functions
#include <iomanip>

// using directives
using namespace std;

// PROGRAM STARTS HERE!
double factorial(double x);
double calculate(double ang);


// Declaration of the main function
int main()
{
    // Declare variables
    double angle;
    double calccos;
    double cosine;

    // Ask for user input
    cout << "Enter the angle in radians: ";
    cin >> angle;

    // Compute the value of cos using a function
    calccos = calculate(angle);

    // Compute the actual value of cos
    cosine = cos(angle);

    // Display the result
    cout << "The calculated cosine of " << angle << " is " << calccos << endl;
    cout << "The C++ cosine of " << angle << " is " << cosine << endl;

    // Exit program.
    system("PAUSE");
    return 0;
}

// Calculate the factorial
double factorial(double x)
{
    if (x <= 1)
    {
        return 1;
    }
    else
    {
        return (x * factorial(x-1));
    }
}

double calculate(double ang)
{
    // Declare the variables
    double temp;
    int count = 0;
    double x;

    // For Loop
    for (int i=1; i<=100; i+=2)
    {
        // Test the values
        if ( i==1 || fabs(x) >.0001)
        {
            if ( count % 2 == 0)
            {
            temp = temp + (pow(ang, i+1)/factorial(i));
            }
            if ( count % 2 == 1)
            {
             temp = temp - (pow(ang, i+1)/factorial(i));
            }

            count++;
            x= pow(ang, i+1)/factorial(i);
        }
    }

    // Display the number of terms in the series
    cout << "There were " << count << " numbers of terms used in the series approximation. " << endl;
    return temp;
}

[Labdabeta]

There are a few problems with your code, but the only critical errors are in the calculate function so I will start there.

First of all when you declare temp and x you dont give them values, so they can be any value they want. This means that when you say:

temp = temp - (pow(ang, i+1)/factorial(i));

The result could, technically, be anything!

Secondly your implementation is extremely redundant. Trace the i in your for loop and you will see that it is always odd, so only one of your if statements will ever execute. Not to mention the fact that you could have merely used an else statement to write the second if.

Finally there is the overall logical error. The pseudo-code for computing cosines is as follows:

Option 1: Count by 2s, store (-1)^n
1. Set the total sum to 0.
2. Set the subtract flag to false. (We add the first term)
3. Set the counter, i, to 0. (This is your for loop)
4. Set temp to the current term: temp=x^i/i!
5. If the subtract flag is true negate temp.
6. Invert the subtract flag
7. Add temp to the sum.
8. Add 2 to i
9. Go to 3

Option 2: Count by 1s, calculate 2n
1. Set the total sum to 0.
2. Set the counter, i, to 0. (This is your for loop)
3. Add (-1^i)x^2i/2i! to the sum.
NOTE: -1^i is 1 iff i is odd
4. Add 1 to i
5. Go to 2

Due to its simplicity I prefer the second option, but both are valid.

Here is valid c++ code for both versions:

Option 1:

#define MAX_ITERATIONS 100
double cos(double x)
{
    //1. Set the total sum to 0.
    double sum=0.0;

    //2. Set the subtract flag to false. (We add the first term)
    bool subtract=false;

    //3. Set the counter, i, to 0. (This is your for loop)
    for (int i=0; i<MAX_ITERATIONS; i+=2)//also does step 8
    {
        //4. Set temp to the current term: temp=x^i/i!
        double temp=pow(x,i)/factorial(i);

        //5. If the subtract flag is true negate temp.
        if (subtract)
            temp*=-1;

        //6. Invert the subtract flag
        subtract=!subtract;

        //7. Add temp to the sum.
        sum+=temp;

        //8. Add 2 to i
        //9. Go to 3
        //covered by for loop
    }
    return sum;
}

Option 2:

#define MAX_ITERATIONS 100

double cos(double x)
{
    //1. Set the total sum to 0.
    double sum=0.0;

    //2. Set the counter, i, to 0. (This is your for loop)
    for (int i=0; i<MAX_ITERATIONS; ++i)
    {
        //3. Add (-1^i)x^2i/2i! to the sum.
        //  NOTE: -1^i is -1 iff i is odd.
        if (i%2==1)                        //if i is odd
            sum-=pow(x,2*i)/factorial(2*i);
        else                               //if (i%2==0) or if i is even
            sum+=pow(x,2*i)/factorial(2*i);

        //4. Add 1 to i
        //5. Go to 2
        //covered by for loop
    }
    return sum;
}

I believe that your solution is using elements from both of these solutions, all you need to do is pick one of them and stick to it and you should be fine.

The rest of the problems with your code are merely optimizations or stylistic changes.

For example instead of for (int i=0; i<100; i+=2) you should store 100 somewhere so that if you ever need to change it, it will be easier.

Also your factorial function would be fairly heavy on the stack for large inputs, you should make it sequential:

double factorial(double x)
{
    double ret=1.0;
    while (x>1.0)
    {
        ret*=x;
        x-=1;
    }
    return ret;
}

however you don't even need to do that much work, since you pass an integer to factorial you could benefit from writing factorial to take an integer:

double factorial(int x)
{
    int ret=1;
    for (int i=1; i<=x; ++i)
        ret*=i;
    return (double)ret;
}

This is helpful since the definition of the factorial on non-integers is complicated (my above factorial function only works on integer values as far as I know).

[mike_2000_17]

I would also point out that you don't need to calculate the factorial from scratch at every iteration and the same for the pow(x) calls, you can just calculate them as you go along with the main loop:

#define MAX_ITERATIONS 100
double cos(double x)
{
    // Set the total sum to 0:
    double sum = 0.0;

    // Create a sign to be flipped at iterations:
    double sign = 1.0;

    // Create an accum. for factorial:
    int fact = 1;

    // Create an accum. for power-of-x:
    double pow_x = 1.0;

    for (int i = 0; i<MAX_ITERATIONS; i+=2)
    {
        sum += sign * pow_x / fact;

        // flip the sign:
        sign *= -1;

        // accum two powers of x:
        pow_x *= x * x;

        // accum two factors of factorial:
        fact *= (i + 1) * (i + 2);
    }
    return sum;
}

Also, I should point out that the Taylor expansion you have there is centered around 0. So, you should not expect it to be a very good approximation as you move away from 0. A trick is to use trigonometric identities to be able to calculate things closer to 0 and then transform it back into the correct range.

[user4678]

Thank you so much for the help and clear explanations. I really appreciate it. One more quick question if you don't mind. To calculate the first 5 terms of the series, I've modified the for loop and it works for values from -pi to pi but after that the numbers are wrong. Any suggestions?

Enter the angle in radians: 6.28
The calculated cosine of 6.28 is 20.8921
The C++ cosine of 6.28 is 0.999995

// Calculate cos
double calculate(double x)
{
    // Declare and initailize variable
    double sum=0.0;

    // For loop
    for (int i=0; i<5; i++)
    {
        // Determine if odd or even
        if ( i%2==1 )                        //if i is odd
        {
            sum = sum - pow(x,2*i)/factorial(2*i);
        }
        else                               //if i is even
        {
            sum = sum + pow(x,2*i)/factorial(2*i);
        }
    }

    return sum;
}

[mike_2000_17]

Like I said, the Taylor series is not going to work very well if you move too far away from 0. In this case, it seems that beyond -pi,pi, it stops working. What you need to do is to bring the angle back within that range, using the identity cos(a + 2*n*pi) = cos(a) for any integer n (positive or negative). In other words, add or subtract 2*pi until you get within the range, like this:

while(x > M_PI)
    x -= 2 * M_PI;
while(x < -M_PI)
    x += 2 * M_PI;

Put that code at the start of your function and it will work. (i.e., 6.28 is going to be near 0).

[user4678]

Okay thank you very much for the feedback. Sorry I am new to programming. Alright, so going back to the initial program. I need it to compute the values of cos as long as the absolute value of a term is greater than 0.0001 and display the # of terms (i). I'm not sure if I did this part correctly. I am getting (lldb) thread breakpoint after it compliles and I run it. I will include all of my code again.

// Pre-processor Directives
#include<iostream> // Required for for use of cin, cout
#include<cmath> // Required for use of sin, cos, pow functions
#include <iomanip>
#define MAX_ITERATIONS 100

// using directives
using namespace std;

// PROGRAM STARTS HERE!
double factorial(double x);
double calculate(double x);


// Declaration of the main function
int main()
{
    // Declare variables
    double angle;
    double calccos;
    double cosine;

    // Ask for user input
    cout << "Enter the angle in radians: ";
    cin >> angle;

    // Compute the value of cos using a function
    calccos = calculate(angle);

    // Compute the actual value of cos
    cosine = cos(angle);

    // Display the result
    cout << "The calculated cosine of " << angle << " is " << calccos << endl;
    cout << "The C++ cosine of " << angle << " is " << cosine << endl;

    // Exit program.
    system("PAUSE");
    return 0;
}

// Calculate the factorial
double factorial(double x)
{
    // Declare and initailize variable
    double ret=1.0;

    while (x>1.0)
    {
        ret = ret*x;
        x = x-1;
    }

    return ret;
}

// Calculate cos
double calculate(double x)
{
    // Declare and initailize variable
    double sum=0.0;
    const double PI = 2*acos(0.0); // PI = arccos(0)*2
    int i = 0;


    // Keep within range of -pi to pi
    // cos(x + 2*int*pi) = cos(x)
    while(x > PI)          // Subtract 2 pi if x > pi
    {
        x = x - 2 * PI;
    }
    while(x < -PI)        // Add 2 pi if x < -pi
    {
        x = x + 2 * PI;
    }

    // For loop
    for (int i=0; i<=MAX_ITERATIONS; i++)
    {
        // Test the values
        if ( i==0 || fabs(x) >.0001)
        {
            // Determine if odd or even
            if ( i%2==1 )                        //if i is odd
            {
                sum = sum - pow(x,2*i)/factorial(2*i);
            }
            else                               //if i is even
            {
                sum = sum + pow(x,2*i)/factorial(2*i);
            }

        }
    }
    return sum;
    cout << " There were " << i << "number of terms used in the series approximation" << endl;
}

[Labdabeta]

Your problem is that when you check that each term is >.0001 you check it against x. Your term is not x, its pow(x,2*i)/factorial(2*i) so you just need to modify your for loop. There is another problem however. The 'i' in your loop will be out of scope on line 96 (which will never be reached because its after a return statement). Also your loop will always loop 100 times because you never told it to 'break'. Here is your corrected loop:

int numIterations;
for (int i=0; i<=MAX_ITERATIONS; i++)
{
    //save the value of the term:
    double term=pow(x,2*i)/factorial(2*i);

    //Test the values
    if (i==0||fabs(term)>0.00001)
    {
        if (i%2==1) //if i is odd
            sum-=term;//a-=b is the same as a=a-b
        else 
            sum+=term;
    }
    else //we are done, we need to save 'i' and leave the loop
    {
        numIterations=i;
        break;
    }
}

cout<<"There were "<<i<<" terms used in the series approximation"<<endl;
return sum;

[user4678]

1 last question (I promise), the program says that there were 0 terms used if I set i=0 before the loop, or 1606416912 terms for 3.14 if I just declare and don't initialize int i in the function. Is this correct or am I missing something?

[Labdabeta]

Can you upload your new code so I can see where the issue is?

[user4678]

// Pre-processor Directives
#include<iostream> // Required for for use of cin, cout
#include<cmath> // Required for use of sin, cos, pow functions
#include <iomanip>
#define MAX_ITERATIONS 100

// using directives
using namespace std;

// PROGRAM STARTS HERE!
double factorial(double x);
double calculate(double x);


// Declaration of the main function
int main()
{
    // Declare variables
    double angle;
    double calccos;
    double cosine;

    // Ask for user input
    cout << "Enter the angle in radians: ";
    cin >> angle;

    // Compute the value of cos using a function
    calccos = calculate(angle);

    // Compute the actual value of cos
    cosine = cos(angle);

    // Display the result
    cout << "The calculated cosine of " << angle << " is " << calccos << endl;
    cout << "The C++ cosine of " << angle << " is " << cosine << endl;

    // Exit program.
    system("PAUSE");
    return 0;
}

// Calculate the factorial
double factorial(double x)
{
    // Declare and initailize variable
    double ret=1.0;

    while (x>1.0)
    {
        ret = ret*x;
        x = x-1;
    }

    return ret;
}

// Calculate cos
double calculate(double x)
{
    // Declare and initailize variable
    const double PI = 2*acos(0.0); // PI = arccos(0)*2
    double sum=0.0;
    double term;
    int numIterations;
    int i;

    // Keep within range of -pi to pi
    // cos(x + 2*int*pi) = cos(x)
    while(x > PI)          // Subtract 2 pi if x > pi
    {
        x = x - 2 * PI;
    }
    while(x < -PI)        // Add 2 pi if x < -pi
    {
        x = x + 2 * PI;
    }


    // For loop
    for (int i=0; i<=MAX_ITERATIONS; i++)
    {

        // Save the value of the term:
        term = pow(x,2*i)/factorial(2*i);

        //Test the values
        if (i==0||fabs(term)>0.00001)
        {
            if (i%2==1) //if i is odd
            {
                sum-=term;
            }
            else
            {
                sum+=term;
            }
        }
        else // Save 'i' and end the loop
            {
                numIterations = i;
                break;
            }
    }

        cout << "There were " << i << " terms used in the series approximation." << endl;
        return sum;
}

[Labdabeta]

cout<<"There were "<<numIterations<<" terms used in the series approximation."<<endl;

The whole point of saving i to numIterations is because outside the for loop 'i' doesn't exist, so its value (if it exists at all) is undetermined. The 'i' you created on line 65 is NOT the same 'i' as the one in the for loop.

[user4678]

Hmmm.. okay now I get the warning "variable 'numIterations' is used unititialized whenever 'for' loop exists because it's condition is false".
It prints 8 terms for pi. and 1 term for 2 pi.

[Labdabeta]

You should initialize it to MAX_ITERATIONS, just in case you never get a term less than 0.00001.