Skip to main content
retract "use FP arithmetic" remarks
Source Link
J_H
  • 35.2k
  • 3
  • 32
  • 129

use FP arithmeticscaled integer

Let's take the "two decimal places"No app-level motivating use case, where 10 ** 2 gives 100. It's easy to generalize was offered in the original submission.

The FP expressionIt is possible that an app which cares about rounding details of a very large set of FPs which includes Math10.round(number * 100)395 will always give you the result youmay want, even if number is "a bit off" a different approach. Turn that into a string and slice it as you like, with a "." dot inLet's focus on the middle"two decimal places" case.

Or divide that FP quantity by 100,Consider advising app developers to obtain awork with integer number of cents, that will still format nicelyrather than FP decimal number of dollars. Format with two decimal places upon display. For FP operations on a restricted range of integers, coming out as you want it conservatively the natural numbers less than a trillion, FP calculations are always exact. Fifty-three bits of significand is a lot of bits to represent exact integer quantities.

use FP arithmetic

Let's take the "two decimal places" case, where 10 ** 2 gives 100. It's easy to generalize.

The FP expression Math.round(number * 100) will always give you the result you want, even if number is "a bit off". Turn that into a string and slice it as you like, with a "." dot in the middle.

Or divide that FP quantity by 100, to obtain a number that will still format nicely, coming out as you want it.

scaled integer

No app-level motivating use case was offered in the original submission.

It is possible that an app which cares about rounding details of a very large set of FPs which includes 10.395 may want a different approach. Let's focus on the "two decimal places" case.

Consider advising app developers to work with integer number of cents, rather than FP decimal number of dollars. Format with two decimal places upon display. For FP operations on a restricted range of integers, conservatively the natural numbers less than a trillion, FP calculations are always exact. Fifty-three bits of significand is a lot of bits to represent exact integer quantities.

Source Link
J_H
  • 35.2k
  • 3
  • 32
  • 129

document the return value

round(number, decimalsLength) {

The name suggests something similar to this documented function:

Math.round()

The Math.round() static method returns the value of a number rounded to the nearest integer.

That's a crystal clear guarantee about the return value.

Your function offers no assurances at all about what's returned.


document the input value

var number = (typeof number === "number") ? number : (number != null) ? parseFloat(number) : 0; // Return NaN for [null; {}]

Your round() is a total function. Silently accepting any and all types for what ostensibly should be a number, and mapping a subset of them to 0, sounds disastrous for the caller.

Much better to insist on one-or-more input types, perhaps FP and string, and throw a fatal error if caller messed up by passing in some random object.


be direct

var right = "1"+(array[1] || 0); // We add 1 to keep the left 0

Hacks such as this do not lead to maintainable code.

Say what you mean and mean what you say. Starting with "234.567" and then assigning right = "1567" tells the reader that right does not mean "the digits to the right of the decimal point". We're left with the identifier not having a clear meaning. Break this out as a well-named helper if there's a transform you need.


solve a general problem

if(right.length-1 >= decimalsLength) {
    ...
    if(parseInt(right.substr(decimalsLength+1, 1)) >= 5)
        right = ... [an integer expression, definitely not a string] ...

    if(right.toString().substr(0, 1) == 2) { 
        ...
        return ...
    }
}
...
return ... [expression which involves 'right'] ...

Wow, that's a lot of special cases.

First, extract helper, which would give you an opportunity to name and to document that you're transforming from this to that. Plus you could separately unit test the helpers.

Second, consider using FP arithmetic to solve this problem. Your chief complaint seems to be that FP quantities describe intervals on the real number line, and some are "a bit off", a bit negative. You give the example of 10.395, which encodes an interval that contains 10.395, but which starts at 10.394999...99905, leading to unfortunate truncated string results. For restricted input ranges, which you'd need to document, you might be able to get away with always adding epsilon = 1e-12 to the input number, and then rounding, obviating the need for many ifs.

Obtaining 100% code coverage via unit tests would pose some slight challenge, given the current code.


use FP arithmetic

Let's take the "two decimal places" case, where 10 ** 2 gives 100. It's easy to generalize.

The FP expression Math.round(number * 100) will always give you the result you want, even if number is "a bit off". Turn that into a string and slice it as you like, with a "." dot in the middle.

Or divide that FP quantity by 100, to obtain a number that will still format nicely, coming out as you want it.


As a consumer of this code, I would not want to call into it, as it is unclear what value it might return.

As a QA tester, I would not want to be tasked with obtaining 100% code coverage. (The OP, alas, did not include any automated testing.)

As a maintenance engineer on the team responsible for this library routine, I would not be able to confidently add features or fix bugs, as the current contract is unclear. Recommend revising this code before attempting to merge it down to main.