Function to format numbers with thousand separators and decimals

Question

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.

Answer 1

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;

Answer 2

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 written typeof numToFormat === "number";

• Use isNaN(number) to determine if a variable is "Not a Number"

• Use Number(val) to convert to a number rather than parseFloat(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. Maybe formatNumber would be better

• Spaces between operators make the code more readable. nextLoop = i+3; as nextLoop = i + 3;

• Space between if and ( also for ( , 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 and decimalSep

• 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];
}