I've wroted this function because I was getting trouble using toLocaleString()
in google-apps-script, and after some refactorings I got to a working code:
function neat(numToFormat,prefThousands, prefDecimals) {
var thousandSep = prefThousands == undefined ? "." : prefThousands;
var decimalSep = prefDecimals == undefined ? "," : prefDecimals;
function parseInput(numToFormat) {
var isNumber = typeof(numToFormat) == 'number';
var numberToFixed;
if(isNumber){
numberToFixed = numToFormat.toFixed(2);
} else {
//replacing "," for parseFloat precision
if(numToFormat.indexOf(",") >-1){
numToFormat = numToFormat.replace(",",".");
}
numberToFixed = parseFloat(numToFormat).toFixed(2);
}
return numberToFixed;
}
function placeThousands(wholesPart){
/* To add thousands separators.
* it iterates throught the
* number in chunks of len 3.
* adding a the setted prefered thousand separator.
* just needed if wholesPart.length >=4
*
*/
var wholesLength = wholesPart.length;
var wholesModulo = wholesLength % 3;
//benefits from the mod equaling the slice end size needed
//and if mod == 0 fstChunk = ""
var fstChunk = wholesPart.slice(0,wholesModulo);
var placed =wholesModulo !=0 ? fstChunk+thousandSep : fstChunk;
for (var i=wholesModulo;i<wholesLength;i+=3) {
var nextLoop = i+3;
if(nextLoop<wholesLength) {
var sliceBy = wholesPart.slice(i,nextLoop);
placed += sliceBy +thousandSep;
} else {
sliceBy = wholesPart.slice(i,wholesLength);
placed += sliceBy;
}
}
return placed;
}
var numberToFixed = parseInput(numToFormat);
var decimalPart = numberToFixed.slice(-3).replace('.',decimalSep);
var wholesPart = numberToFixed.slice(0,(numberToFixed.length - 3));
var needThousands = wholesPart.length >=4;
var neat = needThousands ? placeThousands(wholesPart) + decimalPart : wholesPart + decimalPart;
if(numberToFixed>= 9007199254740991){
neat= "This number is too big, neatnumbers can't handle it";
} else if(neat == "NaN") {
neat= "neatnumbers only deals with numbers";
}
return neat;
}
It's restricted to 9007199254740991 because calling parseFloat()
even on integers bigger than that is imprecise.
Speed is not an issue, I coded solo, so commenting is somewhat distressing since I'm afraid to restate the code. But I'm open for criticizing on any aspects.
2 Answers 2
You can use default parameters to automatically assign a value if none is given.
function neat(numToFormat, thousandSep = ".", decimalSep = ",")
You should start by checking if the input is valid, instead of doing it at the end. It would also be better to throw an error, since it's not easy to tell from the result if the formatting succeeded or not.
if (typeof numToFormat == 'number') {
if(Number.isNaN(numToFormat) || !Number.isFinite(numToFormat)) {
throw new Error("Invalid number");
}
}
else if(typeof numToFormat == 'string') {
if(!/^\d+((,|\.)\d+)?$/.test(numToFormat)) {
throw new Error("Invalid string");
}
}
else {
throw new Error("Can only use number or string");
}
It's not parseFloat()
which is imprecise, but the numbers themselves. JavaScript uses IEEE 754, which is 64-bit floating point numbers, with 53 bits used for precision. The number you are testing against is the highest safe integer, which is 2^53-1 and can be gotten from Number.MAX_SAFE_INTEGER
. But your result could still be imprecise with numbers this large, since you are not only using integers but also 2 decimal places.
With string inputs you can get around this by not converting it to a number at all. Since the result is also a string, you can keep it a string all the way through. For number inputs you can only work with the precision you've been given. Any imprecision should be handled by the code calling this function beforehand.
You will need to update your parseInput
function to avoid parseFloat
. Just cut of any extra decimals, or add them if they are missing. If you want to round the number it gets a bit harder, but it's still possible.
Your placeThousands
function can be done a little simpler. There are several ways to do it, but I would cut the string into an array of chunks and then join
them together. I would also make it easier to change the chunk size by putting it in a variable.
const chunkSize = 3;
let chunks = [];
let start = wholesPart.length % chunkSize;
if(start != 0) {
chunks.push(wholesPart.slice(0, start));
}
for(let i = start; i < wholesPart.length; i+= chunkSize) {
chunks.push(wholesPart.slice(i, i + chunkSize));
}
return chunks.join(thousandSep)
When I saw fstChunk
the first thing I thought was 'fast chunk', but I guess it's supposed to be 'first chunk'. There is no reason make things less clear just to save 2 characters.
You don't need to check the length of wholesPart
, placeThousands
can handle short strings.
return placeThousands(wholesPart) + decimalPart;
-
\$\begingroup\$ Thank you @Kruga, unfortunatelly, Default parameters and Number.MAX_SAFE_INTEGER don't work on google-apps-script, but the other points are valid and welcomed.
fstChunk
really is bad :( \$\endgroup\$Celso Pereira Neto– Celso Pereira Neto2019年04月26日 00:03:36 +00:00Commented Apr 26, 2019 at 0:03
Style and code
For max integer value use
Number.MAX_SAFE_INTEGER
Use
===
rather than==
and!==
rather than!=
typeof
is not a function.typeof(numToFormat) == 'number';
can be writtentypeof numToFormat === "number";
Use
isNaN(number)
to determine if a variable is "Not a Number"Use
Number(val)
to convert to a number rather thanparseFloat(val)
If you are going to exit on
NaN
or out of range do it early rather than after you process everything.If a variable is not going to change define it as a constant with
const
neat
is a rather ambiguous name for the function. MaybeformatNumber
would be betterSpaces between operators make the code more readable.
nextLoop = i+3;
asnextLoop = i + 3;
Space between
if
and(
alsofor (
,while (
and other tokens followed by(
. And space between) {
Logic
The whole thing feels over complicated. For values under 1000 all you do is maybe replace the decimal point. For other values you need only handle the thousands separator.
There is also the issues of negative numbers. You don't return a string saying No can do. The value you return will have a 1000 separator in the wrong place if the number length is divisible by 3 eg neat(-100)
returns "-.100,00"
Rewrite
The rewrite fixes the negative number problem.
I added a places variable so that the number of decimal places can be set.
Uses default parameters to define defaults for (new
places
arg),thousandSep
anddecimalSep
Rather than return error string I return the number argument. It is likely that if the value overflows or is not a number the calling code will not check if the result is one of the error strings. This way what goes in will still have meaning when it comes out.
Code
function formatNumber(number, thousandSep = ",", decimalSep = ".", places = 2) {
if (isNaN(number)) { return number }
var result = number < 0 ? "-" : "";
number = Math.abs(number);
if (number >= Number.MAX_SAFE_INTEGER) { return result + number.toFixed(places) }
var place = Math.ceil(Math.log10(number));
if (place < 3) {
return result + number.toFixed(places).replace(".", decimalSep);
}
while (place--) {
result += number / 10 ** place % 10 | 0;
if (place > 0 && place % 3 === 0) { result += thousandSep }
}
return result + decimalSep + number.toFixed(places).split(".")[1];
}
-
\$\begingroup\$ Thank you @Blindman67, negative numbers really escaped me. Didn't test them at all. I liked the reasoning of returning always a number. \$\endgroup\$Celso Pereira Neto– Celso Pereira Neto2019年04月26日 00:31:57 +00:00Commented Apr 26, 2019 at 0:31
Explore related questions
See similar questions with these tags.