patterncppMinor
Rounding decimals
Viewed 0 times
decimalsroundingstackoverflow
Problem
I've decided to write a small function for rounding decimals because it'll most likely be useful later on. However, my current attempt seems pretty inefficient. How would one best optimize such a function?
#include
double getRemainder(double a, double b)
{
int quotient = (int)(a/b);
double remainder = a - (quotient * b);
return remainder;
}
double roundDecimal(double number, int pos)
{
double factor = pow(10.0, pos);
double factorPlusOne = factor*10;
int importantDigit = (int)(number*factorPlusOne) % 10;
if (importantDigit < 5)
{
return number - getRemainder(number, 1/factor);
}
else
{
double extraPart = (1/factor) - number + number - getRemainder(number, 1/factor);
return number + extraPart;
}
return importantDigit;
}Solution
I agree with the comments implying that floating-point shouldn't be used with money due to issues with precision. Nonetheless, I'll point out some general flaws I've found in your code.
-
There should be a
-
You're casting the C-way. Cast the C++ way with
-
This doesn't quite work as expected:
Here, the last
I've tested this anyway by commenting out the last
-
There should be some input validation to prevent
According to my tests, passing in 10.878 for
-
-
There are some unneeded calculations here:
The
-
There should be a
std:: in front of pow(10.0, pos) since you're not using using namespace std. This is not always caught by some compilers, but it's still best to do this.-
You're casting the C-way. Cast the C++ way with
static_cast:int quotient = static_cast(a/b);int importantDigit = static_cast(number*factorPlusOne) % 10;-
This doesn't quite work as expected:
if (importantDigit < 5)
{
return number - getRemainder(number, 1/factor);
}
else
{
double extraPart = (1/factor) - number + number - getRemainder(number, 1/factor);
return number + extraPart;
}
return importantDigit;Here, the last
return statement will never be reached. This is because either of these blocks will execute and return. Since they both return, the function cannot continue past them.I've tested this anyway by commenting out the last
return and comparing it to the past results. Assuming the function still gives you the intended results, that return should be removed.-
There should be some input validation to prevent
roundDecimal() from passing in a negative value for pos. It should only accept non-negative values (which includes 0).According to my tests, passing in 10.878 for
number and 0 for pos gives 11, which seems correct. Passing in -1 gives 10 and anything less than that gives 0, which are both not ideal results. This may not crash the program, but they still shouldn't be accepted.-
getRemainder()'s arguments should be validated to prevent division by 0. It would be best to prevent such division from happening, such as with a conditional statement somewhere. You could instead throw an exception at division and handle it if you cannot validate beforehand.-
There are some unneeded calculations here:
double extraPart = (1/factor) - number + number - getRemainder(number, 1/factor);The
numbers just cancel each other out, so they can be removed:double extraPart = (1/factor) - getRemainder(number, 1/factor);Code Snippets
int quotient = static_cast<int>(a/b);int importantDigit = static_cast<int>(number*factorPlusOne) % 10;if (importantDigit < 5)
{
return number - getRemainder(number, 1/factor);
}
else
{
double extraPart = (1/factor) - number + number - getRemainder(number, 1/factor);
return number + extraPart;
}
return importantDigit;double extraPart = (1/factor) - number + number - getRemainder(number, 1/factor);double extraPart = (1/factor) - getRemainder(number, 1/factor);Context
StackExchange Code Review Q#9324, answer score: 5
Revisions (0)
No revisions yet.