Reputation: 31
I'm writing a program to reduce a fraction to the lowest term. Here is my program:
#include <stdio.h>
int main(){
int m,x,n,gcd;
printf("Enter a fraction: ");
scanf("%d/%d",&n,&m);
if(m==0)printf("Error");
else
for(;;){
x=m%n;
if(x==0){
gcd=n;
m/=gcd;
n/=gcd;
printf("In lowest terms: %d/%d",n,m);
}else
m=n;
n=x;
}
return 0;
}
I use Euclid's algorithm to calculate the GCD. When executed, it reports
Floating point exception (core dumped)
What's wrong with my code?
Upvotes: 3
Views: 412
Reputation: 31
@paddy
Yes, I dint't see it through. Anyway my code has been flawed from the beginning. I use for(;;)
to create loop but I tell it to stop at else
. It is completely flawed because the loop will execute again and create an infinte loop. Additional, I intended to assign gcd
to n
, but before that n
was assigned to x
(that case,after else
, the loop executed). Therefore if x
is 0
then m/=gcd
and n/=gcd
may have caused core dumped
. This is a stupid code that I shouldn't have asked. So I thought again and rewrote the code.
#include <stdio.h>
int main(){
int m,x,n,gcd;
int M,N;
printf("Enter a fraction: ");
scanf("%d/%d",&n,&m);
M = m;
N = n;
if(m==0)printf("Error");
else
do{
x=m%n;
m=n;
n=x;
}while(x!=0);
gcd=m;
M/=gcd;
N/=gcd;
printf("In lowest terms: %d/%d\n",N,M);
return 0;
}
Because after the loop
, m
and n
has been modified so m/=gcd
and n/=gcd
can't return accurate result. So I assign n
to N
,m
to M
so that the value I entered won't change. Assigning gcd
to m
is a bit tricky, but it's not wrong because before that m
was assign to n
. so I think this trick would have same effect. I use do while
loop because I can set the "termination point", so there will be no infinite loop
.
Upvotes: 0
Reputation: 63481
You have fallen victim to block scope here:
if(x==0){
// ...
}else
m=n;
n=x;
This will be executed as:
if(x==0){
// ...
}else{
m=n;
}
n=x;
That means when x
is zero, then n
gets set to zero at the end of the loop. When you come around again to calculate m % n
, you have a problem. Or if for some reason that gives a value without crashing your program, then gcd = n
will end up giving you divide-by-zero when you subsequently divide m
and n
by gcd
.
If you want more than one statement to be part of the else
branch, then you need to enclose them in braces as follows:
if(x==0){
// ...
}else{
m=n;
n=x;
}
In fact, it's good practice to always use the braces, even when you only have one statement. At my workplace this is even in our coding standards, and part of the code review process.
Upvotes: 5