openexchangerates / accounting.js

A lightweight JavaScript library for number, money and currency formatting - fully localisable, zero dependencies.
http://openexchangerates.github.io/accounting.js
MIT License
4.95k stars 528 forks source link

updating formatColumn to determine if first argument is a string value. #158

Closed jjsquillante closed 7 years ago

jjsquillante commented 7 years ago

@josscrowcroft Great job on accounting.js. It's been a pleasure reading through the code. Please let me know if I can send some money to buy you a cup of coffee or beer!

As for the pull request: When a string value is passed as the first argument, every character is returned as a value in the array which can cause unwanted results.

accounting.formatColumn('1,234.50')
[$1.00,$0.00,$2.00,$3.00,$4.00,$0.00,$5.00,$0.00]

According to your comments in the code and the qunit tests, the first argument should only be an array of numbers.

I propose adding a check to detect if the first argument is a string and, if so, return an empty array. This matches scenarios like if a number, function, or object is passed as the first argument. (all return an empty array)

jjsquillante commented 7 years ago

closed to submit other PR (more logically sound).