Skip to main content
1 of 2
J_H
  • 35.3k
  • 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.

J_H
  • 35.3k
  • 3
  • 32
  • 129