3

I have employed a recursive call to a closure in order to mitigate against some race conditions I am getting (That's what I think it is). I'd like to know if (and why) this is a good solution or a bad solution.

function getResources(activity, resource) {
    var q = when.defer(),
    resourceQuery = constructSelectionQuery(resource),
    timeoutCounter = 0,
    timeoutLimit = 100;
    function queryDatabase() {
        query.getResult({
            query: resourceQuery
        }, function(err, result) {
            if (result.length === 0) {
                if (timeoutCounter > timeoutLimit) {
                    q.reject();
                }
                timeoutCounter++;

                //subsequent call(s)
                queryDatabase();
            } else {
                q.resolve(result);
            }
        });
    }

    //initial call
    queryDatabase();

    return q.promise;

    }

What I am trying to ascertain is the validity of using recursion and closure in this combination - is this an acceptable way of approaching the problem of repeating the call to the database? Without the repeating of the call the result is sometimes empty. This is curious as this function is only called after the record has been saved. However this is in another system so I am assuming the speed of indexing is at fault in MySQL - I don't want to go into that aspect of the system as I'm happy with what I've got - My question is isolated to using this pattern - a recursive call to the closure as a way to repeat the call.

0

1 Answer 1

1

The nesting is a little hard to read. If you write it as follows, it's easier to tell what's happening.

function queryDatabase() {
    query.getResult({query: resourceQuery}, queryDone);
}

function queryDone(err, result) {
    if (result.length !== 0) {
        q.resolve(result);
    }
    else if (timeoutCounter++ > timeoutLimit) {
        q.reject();
    }
    else {
        queryDatabase();
    }
}

Since query.getResult() is asynchronous (I'm assuming, or else you wouldn't need a promise), that means queryDatabase() will return before queryDone() is called. When queryDatabase() is called inside queryDone(), it will return, causing queryDone() also to return, before it's called again. So while it's recursive in a semantic sense, the function calls themselves are never nested. So you shouldn't have issues like keeping 100 copies of a result object in memory at once.

As far as style goes, I don't see how it could be expressed without recursion without adding complexity. If this pattern is repeated frequently in your code, you could make a more general solution, used something like:

var promise = 
  repeatQueryUntil(resourceQuery, function(err, result) {return result.length !== 0;})
1
  • Thanks Karl, this is just the sort of feedback I was looking for. The part about '...keeping 100 copies...' was the bit I was concerned about - however my understanding isn't advanced enough. Could you possibly expand on the '...the function calls themselves are never nested...' Or point me in the direction of something that might explain that aspect. Also your improvements are great - It's hard to get feedback like this when you work on your own!
    – Stevo
    Apr 2, 2014 at 19:27

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.