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.
\$\endgroup\$
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.

\$\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.

\$\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\$ – Stephan Bijzitter Feb 1 '16 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\$ – Musa Haidari Feb 2 '16 at 9:57

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.