HiveBrain v1.2.0
Get Started
← Back to all entries
patterncppMinor

Rounding decimals

Submitted by: @import:stackexchange-codereview··
0
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 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.