1
#include <stdio.h>
#include <math.h>
#define PI 3.1416
double fact(n){
  double x = 1;
  for(int i = 1 ; i <= n ; i++){
    x = x*i;
  }
  return x;
}
int main(void) {
  int deg,term,n=1,sign=1;
  float radian,result=0;
  printf("Enter the Angle (in degree) : ");
  scanf("%d",&deg);
  printf("Enter the number of terms : ");
  scanf("%d",&term);
  radian = deg*(PI/180.0);

  for(int count = 0 ;n<=term ; count+=2){
    result = result + sign *(double)(pow(radian,count)/fact(count));
    n++;
    sign = sign * (-1) ;
  }
  
  printf("user defined cos(%d) = %f\n",deg,result);
  printf("inbuilt cos(%d) = %f\n",deg,cos(deg));
  return 0;
}

I tried similar code with sin function and with different value for count but its not working for cos. If anybody knows why its printing wrong answer... please reply

Some programmer dude
  • 400,186
  • 35
  • 402
  • 621
Naveen
  • 13
  • 2
  • 3
    What do you mean by "not working"? Doesn't it build? Does it crash? Does it give wrong results? Please [edit] your question to add more details. Also please take some time to read [the help pages](http://stackoverflow.com/help), take the SO [tour], read [ask], as well as [this question checklist](https://codeblog.jonskeet.uk/2012/11/24/stack-overflow-question-checklist/). – Some programmer dude Feb 27 '23 at 13:58
  • 1
    OT: Don't use `float` unless you have a very good reason... Here there doesn't seem to be a good reason so change `float` to `double` – Support Ukraine Feb 27 '23 at 14:08
  • 1
    Do not use `pow()` for computing small integer powers. Simple multiplication is faster and sometimes more precise. – John Bollinger Feb 27 '23 at 14:19
  • 3
    You can avoid `pow()` and `fact()` by using a 'working term' that you multiply by the angle and divide by the count in each iteration. – Weather Vane Feb 27 '23 at 14:23

2 Answers2

3

Your code is right, your test is wrong:

Instead of cos(deg), it should be cos(radian).

Moreover, instead of defining PI, you could use the one given in math.h: M_PI:

#define _USE_MATH_DEFINES
#include <math.h>

// from here, you can use M_PI 


You can also improve your code

  1. Since cosine function is periodic and the Taylor series is better near 0, you should clamp the input number in ]-180, 180] range

  2. Factorial function could be computed faster: you have to compute 2!, 4!, 6!... if you store 4! for instance, 6! can be computed with only 2 multiplications instead or recomputing from the start (like you do for sign instead of calling pow(-1, n)

  3. Same for the x^(2n)

Mathieu
  • 8,840
  • 7
  • 32
  • 45
  • 5
    Some implementations provide `M_PI`, but the C language specifications say nothing about it. I think that's a good enough reason to provide one's own `PI`, but a more precise value than appears in the OP's code would certainly be in order. – John Bollinger Feb 27 '23 at 14:14
-1

Your code is inefficient. You call factorial over and over without memoizing it.

Here are better examples in Python. No calls to a factorial function needed. You should be able to port them.

def cosine(t, n):
    result = 0.0
    term = 1.0
    sign = 1.0
    for i in range(0, n):
        result += sign*term
        sign *= -1.0
        term *= t*t/(2*i+1)/(2*i+2)
    return result

def sine(t, n):
    result = 0.0
    term = t
    sign = 1.0
    for i in range(1, n):
        result += sign*term
        sign *= -1.0
        term *= t*t/(2*i)/(2*i+1)
    return result
duffymo
  • 305,152
  • 44
  • 369
  • 561