Return to Answer

• 1

• 22.8k
• 2
• 16
• 40

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