2
\$\begingroup\$

I want to import a CSV file and do some operation on it.

The following is the code I'm using and it is working fine:

 return function (req, res) {
 var rl = require('readline').createInterface({
 input: fs.createReadStream(myFile.csv)
 });
 var lineNumber = 1;
 rl.on('line', function (line) {
 // Validate the first line
 if (lineNumber === 1) {
 if (firstLineIsInvalid()) {
 res.send({
 status: 'error',
 message: 'This file deos not have a proper header'
 });
 rl.close();
 }
 lineNumber++;
 return;
 }
 // Validate the 2nd line
 if (isSecondLineInvalid()) {
 res.send({
 status: 'error',
 message: '2nd line must be blank'
 });
 rl.close();
 lineNumber++;
 return;
 }
 // Skip 3rd line
 if (lineNumber === 3) {
 lineNumber++;
 return;
 }
 // Skip the last line, we do not need aggregations
 if (line.indexOf('Total') > -1) {
 lineNumber++;
 return;
 }
 // Start processing
 processLine(line);
 lineNumber++;
 });
 rl.on('close', function () {
 if (!res.headersSent) {
 fs.rename(myFile.csv, archivedFile.csv, function (err) {
 if (err) {
 res.send({status: 'error', message: 'Archiving failed, '+ JSON.stringify(err)});
 } else {
 res.send({status: 'ok'});
 }
 })
 } else {
 fs.rename(myFile.csv, rejectedFile.csv)
 }
 res.end();
 });
}

But I am wondering if there is a better approach for:

  • Counting the line numbers and validating the lines based on their position in the file.
  • Reacting to the errors, especially those I get in the validation of the first and second lines.
Jamal
35.2k13 gold badges134 silver badges238 bronze badges
asked Oct 31, 2015 at 11:51
\$\endgroup\$

2 Answers 2

2
\$\begingroup\$

Your code is fine and the approach will work. However, I do have the following comments based on my own experience.

  1. In the case of error response, why not use the error object to capture the error details and pass return next(err), something to the effect of:

    //handling errors in your code woud look like
    if(somethingisntright()){
     err.message = 'Something is wrong';
     next(err);
    }
    //error handler presumably in your app.js
    app.use(function(err, req, res){
    //if you have multiple errors you could articulate based on assigning err.status
     res.json({
     status: 'error'
     message: err.message,
     });
    });
    

    This will reduce the amount of error handling code in the current module and centralize error handling thus reducing maintenance effort in the future.

  2. I'm not sure if it is worth it but you may want to consider short circuiting your logic so that the code handling line numbers>= 3 are handled first. It saves some cycles having to evaluate the first two scenarios for every other line regardless.

Jamal
35.2k13 gold badges134 silver badges238 bronze badges
answered Nov 5, 2015 at 22:52
\$\endgroup\$
0
\$\begingroup\$

My answer assumes that myFile.csv's content is not changing.

Currently, you read and parse this file every single time a valid request is made. Now, if the file changes a lot, that is fine. Although you might want to process the changes as it changes, and let Express send back the already-processed data.

If the content of the file does not or does not frequently change, I would highly recommend you process this file whenever it becomes available; probably when you start the server.

answered Feb 1, 2016 at 20:17
\$\endgroup\$
2
  • 1
    \$\begingroup\$ Shameless plug: You can also use a package I wrote, NFP. It will essentially act as a JSON.parse, but for CSV. As this is a library for multiple file-types, it will not increase performance, but it does clean up the code a bit. \$\endgroup\$ Commented Feb 1, 2016 at 20:19
  • \$\begingroup\$ Well, actually the contents of the file are the daily attendance of some staff per mili-seconds, and I do not think in my case it would be feasible to do it at a specific time, since before doing it, the one who uploads it, needs to make sure of some certain things, anyway, thank you for input. \$\endgroup\$ Commented Feb 2, 2016 at 9:57

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

By clicking "Post Your Answer", you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.