X

Aggregator

CodeSOD: Round Strips

The Daily WTF
2 months 3 weeks ago

JavaScript is frequently surprising in terms of what functions it does not support. For example, while it has a Math.round function, that only rounds to the nearest integer, not an arbitrary precision. That's no big deal, of course, as if you wanted to round to, say, four decimal places, you could write something like: Math.floor(n * 10000) / 10000.

But in the absence of a built-in function to handle that means that many developers choose to reinvent the wheel. Ryan found this one.

function stripExtraNumbers(num) { //check if the number's already okay //assume a whole number is valid var n2 = num.toString(); if(n2.indexOf(".") == -1) { return num; } //if it has numbers after the decimal point, //limit the number of digits after the decimal point to 4 //we use parseFloat if strings are passed into the method if(typeof num == "string"){ num = parseFloat(num).toFixed(4); } else { num = num.toFixed(4); } //strip any extra zeros return parseFloat(num.toString().replace(/0*$/,"")); }

We start by turning the number into a string and checking for a decimal point. If it doesn't have one, we've already rounded off, return the input. Now, we don't trust our input, so if the input was already a string, we'll parse it into a number. Once we know it's a number, we can call toFixed, which returns a string rounded off to the correct number of decimal points.

This is all very dumb. Just dumb. But it's the last line which gets really dumb.

toFixed returns a padded string, e.g. (10).toFixed(4) returns "10.0000". But this function doesn't want those trailing zeros, so they convert our string num into a string, then use a regex to replace all of the trailing zeros, and then parse it back into a float.

Which, of course, when storing the number as a number, we don't really care about trailing zeros. That's a formatting choice when we output it.

I'm always impressed by a code sample where every single line is wrong. It's like a little treat. In this case, it even gets me a sense of how it evolved from little snippets of misunderstood code. The regex to remove trailing zeros in some other place in this developer's experience led to degenerate cases where they had output like 10., so they also knew they needed to have the check at the top to see if the input had a fractional part. Which the only way they knew to do that was by looking for a . in a string (have fun internationalizing that!). They also clearly don't have a good grasp on types, so it makes sense that they have the extra string check, just to be on the safe side (though it's worth noting that parseFloat is perfectly happy to run on a value that's already a float).

This all could be a one-liner, or maybe two if you really need to verify your types. Yet here we are, with a delightfully wrong way to do everything.

.comment { border: none; } [Advertisement] Keep the plebs out of prod. Restrict NuGet feed privileges with ProGet. Learn more.
Remy Porter