JavaScript moneyFormat() function

Question

Using JSX syntax I wrote a function that takes either a float or an integer as a parameter and returns a formatted string. I'd like any feedback for refactoring the code.

const moneyFormat = (float) => {
 let answer = [];
 let firstHalf;
 let secondHalf;
 // check if whole number
 if (float % 1 === 0) {
 float = Math.trunc(float);
 firstHalf = float.toString();
 secondHalf = '.00';
 } else {
 float = float.toFixed(2);
 const splitNumArr = float.split('.');
 firstHalf = splitNumArr[0];
 secondHalf = '.' + splitNumArr[1];
 }
 let everyThree = 0;
 // push chars into answer array
 for (let i = firstHalf.length - 1; i >= 0; i--) {
 answer.push(firstHalf[i]);
 everyThree += 1;
 // insert space for every three chars 
 if (everyThree % 3 === 0) {
 answer.push(' ');
 }
 }
 // reverse array and add secondHalf
 answer = answer.reverse().join('') + secondHalf;
 return answer;
}
console.log(moneyFormat(2310000.159897)); // '2 310 000.16'
console.log(moneyFormat(1600)); // '1 600.00'

• DarkMatterMatt: I assume you (like me) are a beginner and are making your own code for the things you learn while doing so :) If you were doing this for a production server you would, of course, Google to find stackoverflow.com/questions/149055/…. Just chucking this link here for anyone coming from a search engine looking for a money formatter.

Answer 1

• Bad variable names. dollars and cents would be more appropriate that firsthalf and secondhalf... I mean, c'mon, these variables don't represent "halfs" of anything. float isn't very descriptive either. Perhaps rawvalue would be better, and maybe result_buffer would be better than answer.
• The entire code for whole numbers is unnecessary. When you run toFixed() on a whole number it becomes a float (string) anyway.
• Rather than creating a new variable and arbitrarily swapping between let and const, why not just reassign float (or in my version, raw_value). You won't need it anymore after that anyway.
• You might wanna consider casting it to a float in case a string is passed to the function, in which case your toFixed would fail.
• It doesn't make a hue difference whether you declare your variables at the top or as needed when you're working on a small function, but be consistent. And if you do declare them all at the top, I would suggest doing it in a single statement for a small performance boost. In this case, I would personally just declare them as you go to keep the function concise.
• When you're doing a backwards loop like that you don't need all three statements in the for loop. You could just do for (let i = firstHalf.length; i--;). That said, looping backwards and then reversing (or looping at all for that matter) makes for smelly code. You can just use a little regular expression to chunk the chars up: /.{1,3}/g
• Don't reassign a variable just to return it, just return it. It's less work for the computer that way.

const moneyFormat = (raw_value) => {
 raw_value = parseFloat(raw_value).toFixed(2).split('.');
 let dollars = raw_value[0],
 cents = '.' + raw_value[1],
 format_buffer = dollars
 .split('').reverse().join('') // reverse it
 .match(/.{1,3}/g) // chunk it
 .map(c=>c.split('').reverse().join(''))
 .reverse(); // reverse it again
 return format_buffer.join(' ') + cents;
}
console.log(moneyFormat(2310000.159897)); // '2 310 000.16'
console.log(moneyFormat(1600)); // '1 600.00'
console.log(moneyFormat("1600"));
console.log(moneyFormat("234123.2134123"));

• DarkMatterMatt: Uh, when I run your code snippet your numbers are reversed in each group of three, e.g. moneyFormat(1600) -> 1 006.00
• I wrestled a bear once.: @DarkMatterMatt - lolwut.. it's 2am why am i answering things on stackexchange... fixed it but it's a little more complicated than i would have liked.. hopefully someone else has a better idea
• Gerrit0: It might be worth defining const reverse = str => str.split('').reverse().join('')