Reading transactions from a file and writing them to a database table

Question

I am learning Node.js and I would like to see if I'm using it correctly:

var mysql = require('mysql');
var fs = require('fs');
var connectionProps = JSON.parse(fs.readFileSync('connection.json', 'utf8'));
var connection = mysql.createConnection({
 host: connectionProps.host,
 user: connectionProps.user,
 password: connectionProps.password,
 database: connectionProps.database
});
connection.connect();
function checkErr(err) {
 if (err) {
 connection.end();
 throw err;
 }
}
connection.query('SELECT * FROM TRANSACTION_TYPE', function(err, rows, fields) {
 checkErr(err);
 var typeNameToId = {};
 for (var i = 0; i < rows.length; i++) {
 typeNameToId[rows[i].NAME] = rows[i].ID;
 }
 fs.readFile(process.argv[2] + '.csv', 'utf8', function(err, content) {
 checkErr(err);
 var data = [];
 var lines = content.split(/\r\n?|\n/);
 for (var i = 0; i < lines.length; i++) {
 if (lines[i].trim() === '') {
 continue;
 }
 data.push(lines[i].split(','));
 var typeName = data[data.length - 1][2];
 data[data.length - 1][2] = typeNameToId[typeName];
 var dateTokens = process.argv[2].split('-');
 data[data.length - 1].push(new Date(dateTokens[0], dateTokens[1] - 1, dateTokens[2]));
 }
 connection.query(
 'INSERT INTO TRANSACTION (DESCRIPTION, AMOUNT, TRANSACTION_TYPE_ID, TRANSACTION_DATE) VALUES ?',
 [data],
 function(err) {
 connection.end();
 if (err) {
 throw err;
 }
 }
 );
 });
});

Answer 1

Connection

You get your connection with the standard mysql.createConnection method. OK, but you also use bare JSON sub-fields to initialise it. What if they're not there? You're going to get a ReferenceError whent the call tries to access it. I'd advise checking that and logging the mistake.

function createDatabaseConnection(properties) {
 if(properties.host && properties.user && properties.pass && properties.database) {
 return mysql.createConnection({
 host: properties.host,
 user: properties.user,
 password: properties.password,
 database: properties.database
 });
 }
 else {
 console.log("Could not retrieve all required details to connect to the database.");
 }
}
var connection = createDatabaseConnection(connectionProps);

Naming

There are a few - not many - places where your naming could be better.

• checkErr - you could name this handleErrors, which would reflect slightly better what it does.
• Table names - are your tables really called TRANSACTION_TYPES, all caps? If they are, OK, there's no standard, but if you're only doing it to match the SQL caps style, you shouldn't be.
• typeNameToId- that reads like a method name. Perhaps call thistypeIds`?

Magic arguments

process.argv[2] - how do you know that what you want is always argument number 3? This might be a good point to look at implementing switches on the command line. You seem to be getting a file name here, perhaps you should use -f filename as the switch.

You've also got a number of array accesses which are done with magic numbers, though that's more common and not a massive issue.

Query errors

In your second query:

connection.query(
 'INSERT INTO TRANSACTION (DESCRIPTION, AMOUNT, TRANSACTION_TYPE_ID, TRANSACTION_DATE) VALUES ?',
 [data],
 function(err) {
 connection.end();
 if (err) {
 throw err;
 }
 }
);

You're redefining how you handle errors. If you simply pass a reference to checkErr (or whatever you've called it, by then) as the third argument instead of the function, you get consistent error handling. I notice your method definition here is different to that in checkErr - is there a reason you're closing the connection before checking if the error actually exists this time?

• SBel: The reason that I close the connection before I check for an error is because I want to make sure that the connection will get closed.
• Åna: @SBel so on the other hand: why do it after you check for the error in checkErr?
• SBel: If I will call checkErr and there is no error then the connection won't get closed.
• Åna: Fair enough. Just want to check that it's deliberate, rather than an error.