asafdav / ng-csv

Simple directive that turns arrays and objects into downloadable CSV files
MIT License
573 stars 215 forks source link

Bug in decimal-separator, inclusive fix and explanation. #160

Open niklaszantner opened 8 years ago

niklaszantner commented 8 years ago

Hi, I found a bug concerning the decimal-separator option, when handling data in string format and passing it to the isFloat function.

csv-service.js:

/**
 * Stringify one field
 * @param data
 * @param options
 * @returns {*}
 */
this.stringifyField = function (data, options) {
  if (options.decimalSep === 'locale' && this.isFloat(data)) {
    return data.toLocaleString();
  }

  if (options.decimalSep !== ',' && this.isFloat(data)) {
    return data.toString().replace(',', options.decimalSep);
  }

  if (typeof data === 'string') {
    data = data.replace(/"/g, '""'); // Escape double qoutes

    if (options.quoteStrings || data.indexOf(',') > -1 || data.indexOf('\n') > -1 || data.indexOf('\r') > -1) {
        data = options.txtDelim + data + options.txtDelim;
    }

    return data;
  }

  if (typeof data === 'boolean') {
    return data ? 'TRUE' : 'FALSE';
  }

  return data;
};

/**
 * Helper function to check if input is float
 * @param input
 * @returns {boolean}
 */
this.isFloat = function (input) {
  return +input === input && (!isFinite(input) || Boolean(input % 1));
};

isFloat is simply not returning true on data like '123.123'. That's why in

if (options.decimalSep !== ',' && this.isFloat(data)) {
    return data.toString().replace(',', options.decimalSep);
} 

the if always will evaluate to false with '123.123' also it is a float. So the decimalSep option is ignored and the default not replaced.

A fix for this would be to simply rewrite the isFloat function:

this.isFloat = function (input) {
  return !isNaN(input) && !isInt(eval(input)) && input.toString().length > 0;
};

function isInt(input) {
  return !isNaN(input) && eval(input).toString().length == parseInt(eval(input)).toString().length;
}

You can also find two videos one befor and one after the fix here: before: https://drive.google.com/file/d/0B2gMUEFwlGRfWVo0Wkk5d1BjVUk/view?usp=sharing after: https://drive.google.com/file/d/0B2gMUEFwlGRfXzI5d3Y4Q3J6aUk/view?usp=sharing

roccomuso commented 7 years ago

+1