[fruchtose]

First of all, thank you for taking the time to read over my code. I don't get enough of this.

You're right that 28 lines is pretty ridiculous, but it's because I never abstracted out that code. The playlist code is some of the ugliest in that project, because I got pretty lazy with it. I know it's terrible. The console logging stuff is part of that.

Q actually allows exceptions to cause promise rejections, which is both a nifty feature and a potential curse (e.g. throwing an exception before releasing a resource).

I like the changes you propose (not considering testing), but with slightly different implementation. the DbCommand should be creating its own deferreds, rather than accepting one as a parameter. This kind of promise handling is best left up to DbCommand to implement, rather than the caller. In Q it's easy to chain promises, like so:

    Q.fcall(someFunction).then(function(result) {
      return functionThatReturnsAPromise();
    }).then(function(secondResult) {
      console.log(secondResult);
    }).done();

Also, in a proper redesign, Q's denodeify function can change the whole flow of the execute function entirely:

    DbCommand.prototype.execute = function(query, onComplete) {
      var self = this;
      Q.denodeify(dbConnectionPool.acquire).then(function(connection) {
        self.connection = connection;
        return Q.denodeify(connection.query);
      }).then(function(rows) {
        return onComplete(rows);
      }).finally(function() {
        self.connection && dbConnectionPool.release(self.connection);
      }).done();
    };

Q's denodeify call works with Node.js callbacks which follow the convention that the error is the first argument, and the result all others. denodeify then converts the error into an argument for any calls to Q.fail. Any uncaught errors will be thrown after done() is called.

However, I am not sold on the idea of creating a prototype for DB commands. There's no state that needs to be held, and the code is abstract enough without introducing a prototype.

Again, thanks for the code review. The playlist DB code needs a lot of refactoring, since right now there's too much repetition I've been too lazy to fix. If you want to talk some more, feel free to email me.