0

I'm attempting to write a simple validation script in Node where error codes are added to an error array when certain validation checks fail. To complicate things, I also have a function called translate which makes a database call and returns the full text associated with an error code (translated into the specified language). In order to achieve this, I'm using the async library.

Unfortunately, the following code always results in an empty array, even when validation should fail. I've confirmed independently that my translate function works. Any ideas why this isn't working?

app.post("/user", function(req, res) {

  var error = [];

  if (req.body.username == "") {
    error.push("username_required");
  }

  if (req.body.password == "") {
    error.push("password_required");
  }

  for (var i = 0; i < error.length; i++) {
    error[i] = function(callback) {
      translate(error[i], req.language, function(text) {
        callback(null, text);
      });
    };
  }

  async.parallel(error, function(error, results) {
    res.send(results);
  });

});

translate = function(code, language, callback) {
  var code = database.escape(code);
  var language = database.escape(language);
  database.query("SELECT content FROM text WHERE language = 'en' AND code = " + code, function(error, row) {
    callback(row[0].content);
  });
}
David Jones
  • 10,117
  • 28
  • 91
  • 139
  • This is the infamous closure problem. Javascritp does not have block scope. – SLaks May 29 '13 at 00:31
  • I see. Any suggestions on an alternate approach? – David Jones May 29 '13 at 00:32
  • Wrap the callback in a function that creates a closure around its parameter. – SLaks May 29 '13 at 00:33
  • http://lostechies.com/derickbailey/2011/12/04/achieving-block-scope-with-immediate-functions-in-javascript/ – SLaks May 29 '13 at 00:33
  • Okay, I'm trying to wrap my mind around this. In general, am I thinking about things in the wrong way? Is this something that is common when using Node and JavaScript? – David Jones May 29 '13 at 00:54
  • Personally think you're on the right track. JavaScript scope in that case can be tricky; SLaks is right, a simple function that creates new scope would fix it. However, I'd change `async.parallel` to an `async.map` that uses the *original* `error` array before you turn them into functions. – Michelle Tilley May 29 '13 at 01:27
  • @BrandonTilley: Great idea. I'm trying that now... – David Jones May 29 '13 at 01:29
  • @BrandonTilley: async.map worked perfectly. I went ahead and posted my solution, but I'd be happy to give you the credit if you want to post an answer. – David Jones May 29 '13 at 01:43

1 Answers1

4

As suggested in the comments, async.map did the trick. Here's my final solution:

app.post("/user", function(req, res) {

  var error = [];

  if (!req.body.username) {
    error.push("username_required");
  }

  if (!req.body.password) {
    error.push("password_required");
  }

  async.map(error, function(error, callback) {
    translate(error, req.language, function(text) {
      callback(null, text);
    });
  }, function(error, results) {
    res.send(results);
  });

});
David Jones
  • 10,117
  • 28
  • 91
  • 139
  • If you can modify `translate` to call the callback in traditional Node.js style (`callback(error, result)`), you can make it even shorter: `translate(error, req.language, callback);` – Michelle Tilley May 29 '13 at 02:51