patterncMinor
Converting a decimal into a fraction
Viewed 0 times
convertingintodecimalfraction
Problem
I have written a small program that converts a decimal number into a reduced fraction representation. I also designed this little program to be fairly robust, so that it can give the correct output for various inputs.
For example:
Here is the complete code for the program:
My algorithm is basically to first process the number right to left. As soon as a non-digit is
For example:
./a.out 3,000.375
24003/8
./a.out "3 000,375"
24003/8
./a.out 3.000,375
24003/8
./a.out 3000.375
24003/8Here is the complete code for the program:
#include
#include
#include
int isDigit(char);
void simplify(unsigned long long *, unsigned long long *);
int main(int argc, char *argv[]) {
if (argc = 0; i--, denominator *= 10) {
c = input[i];
if (!isDigit(c)) {
break;
}
previous = numerator;
numerator = (c - '0') * denominator + numerator;
if (previous > numerator) {
return EXIT_FAILURE;
}
}
if (c == '-' || isDigit(c)) {
printf("%llu/1\n", numerator);
return EXIT_SUCCESS;
}
simplify(&numerator, &denominator);
int j, negative = 0;
unsigned long long whole = 0;
for (j = 0; j whole) {
return EXIT_FAILURE;
}
}
}
previous = numerator;
numerator += whole * denominator;
if (previous > numerator) {
return EXIT_FAILURE;
}
simplify(&numerator, &denominator);
if (negative) {
printf("-");
}
printf("%llu/%llu\n", numerator, denominator);
return EXIT_SUCCESS;
}
int isDigit(char c) {
return c >= '0' && c <= '9';
}
void simplify(unsigned long long *numerator, unsigned long long *denominator) {
while (1) {
if ((*numerator % 2 == 0) && (*denominator % 2 == 0)) {
*numerator /= 2;
*denominator /= 2;
} else if ((*numerator % 5 == 0) && (*denominator % 5 == 0)) {
*numerator /= 5;
*denominator /= 5;
} else {
break;
}
}
}My algorithm is basically to first process the number right to left. As soon as a non-digit is
Solution
I think you're being too lenient with your input checking, which results in some unexpected results:
-3,000,0 results in: -3000/1
3,000 results in: 3/1
3 results in 3/1
3-1 results in 1/1
Personally, I'd pick an expected format and then ensure that the input complies with it. If you want to support multiple decimal marks, then I would suggest having a function for each supported format that validates the input and returns it in one common format which is then used by the rest of the code.
argc & argv
In your example you have
Unless you're going to continue through the arguments supplied, appending them to the input it would be better to do something like this:
One declaration per line
This is obviously subjective, but I don't particularly like multiple declarations on a single line. I don't mind it so much when you're declaring several uninitialised variables, however when you're assigning values to the variables it would be better to have each declaration on a new line. For me, denominator and previous get a bit lost on this line:
-3,000,0 results in: -3000/1
3,000 results in: 3/1
3 results in 3/1
3-1 results in 1/1
Personally, I'd pick an expected format and then ensure that the input complies with it. If you want to support multiple decimal marks, then I would suggest having a function for each supported format that validates the input and returns it in one common format which is then used by the rest of the code.
argc & argv
In your example you have
./a.out "3 000,375". You're quoting the space in order to ensure it's all pushed as part of argv[1]. It seems likely that this could be forgotten. If it was, then you would end up with the output of 3/1. Your argc checking only checks for no parameters being supplied:if (argc < 2) {
fprintf(stderr, "Please enter a number to fractionize.\n");
return EXIT_FAILURE;
}Unless you're going to continue through the arguments supplied, appending them to the input it would be better to do something like this:
if (argc != 2) {
fprintf(stderr, "Usage: %s \n", argv[0]);
return EXIT_FAILURE;
}One declaration per line
This is obviously subjective, but I don't particularly like multiple declarations on a single line. I don't mind it so much when you're declaring several uninitialised variables, however when you're assigning values to the variables it would be better to have each declaration on a new line. For me, denominator and previous get a bit lost on this line:
unsigned long long previous, denominator = 1, numerator = 0;Code Snippets
if (argc < 2) {
fprintf(stderr, "Please enter a number to fractionize.\n");
return EXIT_FAILURE;
}if (argc != 2) {
fprintf(stderr, "Usage: %s <number to fractionize>\n", argv[0]);
return EXIT_FAILURE;
}unsigned long long previous, denominator = 1, numerator = 0;Context
StackExchange Code Review Q#134294, answer score: 7
Revisions (0)
No revisions yet.