Parse a CSV file and return an object or array

Question

I recently published a JavaScript module for NPM which is also available on Github by the name of rawiki-parse-csv. The code takes raw CSV data and returns an array. The method has one option to include the first line as a header in which case an object is returned within the array. If the option is not set each line will be returned as an array.

The project includes a Chai test which inputs some sample CSV data and checks the returned output with both options specified. The tests pass without issue and use in an actual application appears to be faultless.

Could someone comment on any issues with this code, potentials for failure, incorrect structure or areas for improvement?

/* The MIT License (MIT) Copyright (c) 2016 Rawikitua Isherwood Parse module creates an object or set of arrays from a csv file by splitting the text at each new line and splitting each line at comma marks.*/ var parse = function (input, option){ try { // output object var data = {}, // output no columns array container = [], // output array records = [], // splits csv data at each new line lines =input.split(/\r\n|\r|\n/), // creates columns by splitting first line of csv columns = lines[0].split(','); // creates objects from csv data if column option included if (option === true){ // loop through each line of csv file for (var l = 1; l <= lines.length-1; l++) { // splits each line of csv by comma var words = lines[l].split(','); // builds object based on column headers for (var cell in columns) { data[columns[cell]] = words[cell]; } // pushes object to output array records.push(data); // resets object in order to add another data = {}; } } else { // creates nested arrays from csv data if column option omitted,false or not true for (var l = 0; l <= lines.length-1; l++) { // splits each line of csv by comma var words = lines[l].split(','); // creates objects from csv data for (var cell in words) { container.push(words[cell]); } // push array to output array records.push(container); // reset array in order to add another container = []; } } // returns output array return records } catch(err){ return err } } module.exports = parse 

Answer 1

Concept

CSV is a somewhat loosely defined concept. The closest thing there is to a specification is RFC 4180.

This parser is rather naïve: it doesn't handle quoting, and as a result it cannot return data that contain strings with literal commas. Maybe that is good enough for your own use, but in my opinion it's not good enough for a publicly published library, at least not without a giant disclaimer.

For an example of a good CSV library that handles multiple dialects, take a look at Python's built-in library.

Implementation

The purpose of the option is not self-evident. A better name might be headerRow. Also, the way you have nearly duplicated the two cases, you might as well write it as two functions instead.

The code crashes on empty input, instead of returning an empty data structure as I would expect.

Why do you catch an exception and convert it to a return value? That defeats the purpose of the exception mechanism, and forces the caller to handle the possibility of very weird "data" resulting from the parsing.

I'm not a fan of var declarations that span multiple lines, since it is easy to unintentionally write something different if you screw up the punctuation. ("use strict" would be a good practice for helping to detect such errors.) Your use is particularly bad because the intervening comment lines and the lack of additional indentation on the subsequent lines make it hard to read correctly.

l <= lines.length - 1 is not idiomatic: the standard way to write a counting for-loop in JavaScript is:

for (var i = 0; i < array.length; i++) 

Why do you "reset" your temporary variables at the end of the loop for reuse in the next iteration? Why not just create it at the top of the loop?

Commenting every line of code is annoying. Most of your comments can just be eliminated.

Use semicolons consistently. You missed one at return records.

Answer 2

Besides the answer of @200_success, I have one further suggestion for improvement: I recommend making the record separator (currently comma) and the line separator (currently new-line) optional parameters, instead of hard-coding them. (If not provided, the default values could still be comma and new-line.) Of course, the separator parameters could be arrays of strings, so that you can provide e.g. more types of new-line.

Answer 3

As well as what others have said:

• Strict mode. Make debugging great again not so stressful: strict mode throws more errors instead of silently failing or leaking global variables. (It also prohibits with.)
• Please don't use a for...in loop to iterate over arrays. Either use the ES6 for...of loop, or use Array.prototype.forEach(). for...in loops are for object properties not array items, so can break if there are unexpected properties.
• Code style: either use semicolons or don't. If you do, be consistent; if you don't, make sure you know the rules. And opening braces always go on the end of the line, not on a new line; "cuddle" else and catch so they're on the same line as the previous closing brace.
• ES6 let and const may help you with scoping: vars go to the top of the function, not the block.