1

I am writing a piece of code to build a schema from DB and based on other examples on SE, I came up with this way to pass down table name all the way to the code where I will process the table fields (I need access to the table name at that time). The schema is coming from MySQL using jugglingdb-mysql.

Is this the most 'elegant' I can do (I would prefer not to have closures in the code as they are written now)?

schema.client.query('SHOW TABLES', function(err, data) {
    if (err) throw err;
    data.forEach(function(table) {
        // closure to pass down table name
        (function(_table) {
            schema.client.query('SHOW columns FROM ' + _table, function(err, rows) {
                if (err) throw err;
                // closure to pass down table name
                (function(_table) {
                    rows.forEach(function(row){
                        // Go wild processing fields
                    })
                })(_table);
            });
        })(table['Tables_in_db'])
    });
});
Who Cares
  • 13
  • 3

1 Answers1

0

Inner functions inherit the variables from the outer function. In this case you dont need all that trouble to pass the variable and the following should be enough:

data.forEach(function(table) {
    var _table = table['Tables_in_db'];
    schema.client.query('SHOW columns FROM ' + _table, function(err, rows) {
        if (err) throw err;
        rows.forEach(function(row){
            // Go wild processing fields
        })
    });
});

The only time you need to do that trick with the immediately invoked function is when you are creating callbacks inside for-loops. Since Javascript scope is function based instead of block-based, for loops dont introduce a new scope and the iteration variable ends up being shared by all the callbacks (it will end up pointing to the last element).

Community
  • 1
  • 1
hugomg
  • 68,213
  • 24
  • 160
  • 246
  • Indeed, your code works as intended, even thought something similar to your code was not working for me earlier. I must have had my assignment in the wrong place. – Who Cares Aug 10 '14 at 15:43
  • Were you using plain for or while loops instead of the forEach method? – hugomg Aug 10 '14 at 15:45
  • Yes, I started with for loop before I realized it is simpler to go with forEach, just did not think I should get rid of closures. – Who Cares Aug 10 '14 at 15:54
  • That was probably it then. The forEach's callback is already a closure that is creating a new scope - no need for an extra one now :) – hugomg Aug 10 '14 at 16:15