Async, callbacks, and closures

After having spent a month or two trying to learn JavaScript, especially functional programming, async, and closures, I finally get it. But I don't know if I'm writing elegant code... Specifically, I'm not sure if I'm creating too many closures, because I don't entirely grasp when contexts remain and when they're marked for garbage collection.

You can probably tell from the example that I'm using node.js -- basically, this is just a piece of test code that connects to a database, gets 20,000 records asynchronously with individual SELECT queries (I'm just doing some profiling, don't worry), and manipulates the data. At the end, it tells me how long it took and closes the MySQL connection.

In order to get a final timestamp and close the connection, I needed something like the async library to keep track of all the various asynchronous functions that are being spawned. It takes all my queries as an array of functions (using a factory called makeQuery()) and then runs a callback to do the cleanup.

Questions:

-
Am I creating a huge amount of contexts by creating and returning the function doQuery() which uses the argument id in the makeQuery() function, thereby causing 20,000 contexts to persist until async.parallel() is run?

-
Is the factory function makeQuery() a good and tidy way to create this array of doQuery() functions? Can you suggest any better way to do it?

```
var mysql = require('mysql');
var radix64 = require('./lib/radix64');
var async = require('async');

var client = mysql.createClient({user: 'root', password: '12345678'});
client.query('USE nodetest');

var startTime = (new Date()).getTime(); // poor man's profiling
var queries = []; // this will be fed to async.parallel() later

var makeQuery = function makeQuery (id) { // factory function to create the queries
return function doQuery (cb) {
client.query('SELECT * FROM urls WHERE id='+id,
function logResults (e, results, fields) {
results

var makeQuery = function makeQuery (id) {

There is no need to make makeQuery a local variable, just use a function declaration.

results = results[0];
results.key = radix64.fromNumber(results.id);

Your just augmenting the object, your not doing anything with it.

Other then that, your not creating new functions in a loop, your using a function constructor. This correct.

Your running all 19000 queries in parallel rather then in waterfall, which is also correct.

Edit:

Question 1:

You need to create 20000 contexts. Because there are 20000 values of i. Worrying about there being 20000 functions is a micro optimisation. v8 optimizes the hell out of your code.

It actually splits your functions into a hidden class seperate from the closure context, and in my memory you simply have 20000 values of i and one value for the function.

Just to remind you of rule 1.


Never underestimate V8

Question 2:

makeQuery as a factory is the correct pattern to use.

The only other optimisation you can do is to write a real SQL query rather then 20000 dummy ones. But I'll ignore that because it's a dummy example.

StackExchange Code Review Q#6101, answer score: 2