7
\$\begingroup\$

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 <cmath>

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;
}
\$\endgroup\$
5
  • 3
    \$\begingroup\$ Because of the way floating point numbers are stored rounding to the nearest decimal does not work that well (as some numbers just can not be represented so you may think you have done the rounding but the value that can't be exactly represented so it adds many extra bits to get the nearest value). So the best idea is to do the rounding as you print the number out (using the stream precision). Alternatively store the value as an integer then put the decimal point into play as you print it out. \$\endgroup\$ Commented Feb 23, 2012 at 18:31
  • \$\begingroup\$ Try: std::cout << std::setprecision(17) << roundDecimal(10.2,2) << "\n"; It should print out 10.20 but actually prints 10.19. But this is also because 10.2 can not be represented exactly. If you do: std::cout << std::setprecision(17) << 10.2 << "\n"; it will print 10.199999999999999 \$\endgroup\$ Commented Feb 23, 2012 at 18:35
  • \$\begingroup\$ But when calculating money, I'd like 2.4354 to be rounded to 2.44 to get more accurate results. Is there really no way to round a floating point number? If so, are there any BigNumber/BigDecimal libraries for c++ like there are for java? \$\endgroup\$
    – rcplusplus
    Commented Feb 23, 2012 at 18:48
  • 2
    \$\begingroup\$ If you're dealing with money, it'll be much easier to use integers and multiply by 100 (ie, count integral pennies, not fractional dollars or whatever). Otherwise, the floating-point inaccurary will anyway be smaller than a 1/2 penny until you do enough calculation to build up a huge error term, so just formatting to 2 d.p should be fine \$\endgroup\$
    – Useless
    Commented Feb 23, 2012 at 19:05
  • 3
    \$\begingroup\$ If you are dealing with currency you should really be using integers. Use the integer to store the exact value (to the lowest currency unit). Then place the decimal point when printing. Yes there are a couple of Big Decimal libraries available a quick google will locate them. Also see: stackoverflow.com/questions/4798777/… \$\endgroup\$ Commented Feb 23, 2012 at 19:18

3 Answers 3

5
\$\begingroup\$

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<int>(a/b);
    

    int importantDigit = static_cast<int>(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);
    
\$\endgroup\$
2
\$\begingroup\$
  1. As getRemainder() is a helper function, you may want to limit its scope with static.

  2. If pos is negative, one could then round to the nearest 10s, 100s, 1000s, ...

  3. The range of double is far wider than int. The (int)(number*factorPlusOne) % 10; is problematic. Better to use round() and fmod() or another approach, something like the following. With typical FP, one cannot achieve exact values like 0.1, but this will provide the closest (or maybe 2nd) closest representable number.

    double factor = pow(10.0, pos);
    double number_scaled =  round(number * factor);  
    double RoundToPosDigits = number_scaled/factor;
    

Note: the various discussion about money and waiting until print-time to do rounding is good advice.

Note: See fegetround() and fesetround() to get explicit control on rounding modes used by round().

\$\endgroup\$
1
  • \$\begingroup\$ Further point about money. If done in whole numbers or hundredths or whatever, there are compelling reasons to round at intermediate steps in a given calculation. A typical financial interest calculation needs rounding before subsequent subtraction, etc. All the calculations may done long before anything is printed (if ever). Without more detail, simple want to add the OP's desire in a valid issue for many applications including financial. \$\endgroup\$ Commented Dec 15, 2013 at 16:27
1
\$\begingroup\$

getRemainder() is unreliable, because doubles can represent a wider range of numbers than ints. If a is much larger than b, then quotient could overflow. The solution is to use fmod() instead, which should be drop-in replacement for getRemainder().

I'm not sure I'm qualified to review roundDecimal(), because proper rounding is a complicated topic. Instead, I encourage you to ask yourself, why do you want to round a double to a certain number of decimal places? If you're using a double to store currency units (e.g. dollars and cents), don't; store an integral number of cents instead. If you want to round numbers to get pretty output, then there's no point in losing precision in your intermediate representation; just use the proper formatting tools in iostream or snprintf() for human consumption.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.