How could I make this code more effective?
static void print_decimal(double decimal, int precision)
{
size_t integer;
while (precision--)
{
decimal *= 100.;
integer = (int)decimal;
if (!precision && (integer % 10) >= 5 && decimal != (double)integer)
integer += 10;
decimal *= .10;
integer /= 10;
printf("%i", integer);
decimal -= integer;
}
}
Example of calling that show me the decimals of a double with precision
print_decimal(65.226, 4)
-
\$\begingroup\$ ken, Code, should you want to compare the output to code that attempts to print an exact conversions. \$\endgroup\$chux– chux2019年01月29日 21:50:00 +00:00Commented Jan 29, 2019 at 21:50
2 Answers 2
Limited range
double decimal, ... integer = (int)decimal;
is undefined behavior for (削除) well over 90% (削除ここまで) about half of all double
as the value is well outside the int
range of INT_MIN...INT_MAX
.
To truncate a double
, use double trunc(double)
.
Imprecision
decimal *= 100.
is not certainly exact. With corner cases near xxxx.xx5, OP's approach generates a nearby, though wrong, answer.
Negative numbers error
print_decimal(-65.226, 4);
--> -1717987571-1932735284-1932735284-1932735283
Code is buggy
print_decimal(65.9999999999999, 4);
--> 6599910
. This is due to the final rounding not accounted for.
to be more effective
Avoid range errors and those due to multiple rounding.
To do this well is a non-trivial problem and requires a fair amount of code. A suitable short answer is possible if the range of double
allowed (OP unfortunately has not supplied a restricted range) is reduced or sprintf()
is callable.
Below is some quick code. It too has imprecision issues noted about, but less so than OP's. Also OP's various failing cases are corrected here.
#include <limits.h>
#include <math.h>
#include <stdint.h>
static void print10(long double ldecimal) {
if (ldecimal >= 10.0) {
print10(ldecimal/10);
}
ldouble mod10 = fmodl(ldecimal, 10);
putchar((int) mod10 + '0');
}
static void print_decimal2(double decimal, unsigned precision) {
if (!isfinite(decimal)) {
return ; // TBD_Code();
}
if (signbit(decimal)) {
putchar('-');
decimal = -decimal;
}
double ipart;
double fpart = modf(decimal, &ipart);
// By noting if there is a non-zero fraction, then this block only needs to
// handle `double` typically in the [0 ... 2^53] range
if (fpart) {
long double pow10 = powl(10, precision);
ldecimal = roundl(decimal * pow10); // use extended precision as able
print10(ldecimal);
} else if (decimal) {
print10(decimal);
while (precision) {
precision--;
putchar('0');
}
} else {
putchar('0');
}
}
Well, there's the obvious stuff, like whitespace and declaring variables at their first point of use. For example:
static void print_decimal(double decimal, int precision)
{
size_t integer;
while (precision--)
{
decimal *= 100.;
integer = (int)decimal;
could usefully be rewritten as
static void print_decimal(double decimal, int precision)
{
while (precision--) {
decimal *= 100.;
size_t integer = (int)decimal;
This spots a bug: you fail to handle negative inputs. If you want integer
to hold a signed int
, you should define it as int
(not size_t
); and vice versa, if you want it to hold an unsigned size_t
, you should be casting (size_t)decimal
(not (int)decimal
).
if (!precision && (integer % 10) >= 5 && decimal != (double)integer)
integer += 10;
This line could use some comments. I imagine it has something to do with rounding?
printf("%i", integer);
It's more idiomatic to use printf("%d", integer)
to print a decimal integer in C (and C++). True, %i
exists, but it's deservedly obscure.
(Also, because of the above bug where you made integer
a size_t
, down here you should be using %zu
to print it. But once you make it an int
, you can go back to using %d
.)
Every compiler will warn you about the format-string mismatch between %i
and size_t
. Are you compiling with -Wall
? (No.) Why not? (There's no reason not to.) So, start compiling with -Wall
today! It'll find your bugs so that you don't have to.
Performance-wise... I conclude from context that you expect this line to only ever print one digit, is that right? That is, your code could benefit from some "design by contract":
assert(0 <= integer && integer <= 9);
printf("%i", integer);
If the assertion is correct, then we can speed up the code by writing simply
assert(0 <= integer && integer <= 9);
putchar('0' + integer);
However, by adding the assertion, we have made the code a little bit testable. We can write some simple tests — just loop over all the floating-point numbers in the world and see if any of them fail the assertion — and indeed it doesn't take long to find one that fails.
print_decimal(0.097, 2);
This prints
010
when it should actually have printed
10
according to my understanding of your intent.
Which reminds me: print_decimal
is a weird name for this function, since it literally does not have the ability to print a decimal point! Even your example print_decimal(65.226, 4)
actually prints 652260
, not 65.2260
. This seems problematic for your prospective users.
So in conclusion:
- Make your code testable.
- Test your code.
- Compile your code with
-Wall
.
-
3\$\begingroup\$ -Wextra is also often helpful. A few of the things from -Wextra can be annoying if you are doing something intentionally - like if you mean to left shift a negative value for the behavior it produces. But I find it still is more helpful than not, and each of its warnings can be individually disabled if you need to be doing something that raises them. The implicit fallthrough warning can be suppressed with just a comment saying /* intentional fallthrough */ \$\endgroup\$Ed Grimm– Ed Grimm2019年01月29日 05:11:57 +00:00Commented Jan 29, 2019 at 5:11