AngularJS recursive function call with $timeout

Question

I have created a recursive function call in AngularJS and just wonder if is there a better way to solve this problem? The code works perfectly.

My code is the following:

function getLocation(query, map, object, delay) {
    var deferred = $q.defer();
    var service = new google.maps.places.PlacesService(map);
    service.textSearch({ query: query }, function (results, status) {
        if (status == google.maps.places.PlacesServiceStatus.OK) {
            var loc = results[0].geometry.location;
            object.lang = loc.lng();
            object.lat = loc.lat();
            deferred.resolve(object);
        } else {
            if (status === "OVER_QUERY_LIMIT") {
                delay += 100;
                deferred.resolve($timeout(function () {
                    var deferred = $q.defer();
                    getLocation(query, map, object, delay).then(function (object) {
                        deferred.resolve(object);
                    });
                    return deferred.promise;
                }, delay));
            } else {
                deferred.reject(status);
            }
        }
    });
    return deferred.promise;
}

I have already tried to replace the following section:

deferred.resolve($timeout(function () {
    var deferred = $q.defer();
    getLocation(query, map, object, delay).then(function (object) {
        deferred.resolve(object);
    });
    return deferred.promise;
}, delay));

with:

$timeout(getLocation(query, map, object, delay).then(function (object) {
    deferred.resolve(object);
}), delay);

but it didn't work (perform the recursive call without timeout).

Could you give me some advice to build a less ugly function to solve this recursive call?

Answer 1

You're almost there. But first, a few code smells:

function getLocation(query, map, object, delay) {
    ...
    service.textSearch({ query: query }, function (results, status) {
        if (status == google.maps.places.PlacesServiceStatus.OK) {
            ...
            object.lang = loc.lng();
            object.lat = loc.lat();
            deferred.resolve(object);
        }
        ...
    ...
}

Your function returns a promise that resolves with an object containing lat and long. However, you are mutating an object called object. The problem here is the unwanted side effect. If you ever want to use this function elsewhere, you have to supply it an unnecessary object. If you also pass in an existing object, you unintentionally introduce lat and long.

if (status == google.maps.places.PlacesServiceStatus.OK) {
  ...
} else {
  if (status === "OVER_QUERY_LIMIT") {
    ...
  }
}

That google.maps.places.PlacesServiceStatus.OK is very long and visually annoying. Consider putting it in a shorter variable. Also, the if statement in the else block can be collapsed as an else if of the first if statement.

Lastly, the deferred pattern is actually an anti-pattern. Wrap your operation in a promise constructor instead of having a dangling deferred.

If I understand correctly, you need to do a location request, and retry on fail. You simply call your API. Resolve and reject accordingly.

function getLocation(query, map, retryTimeout){
  var service = new google.maps.places.PlacesService(map);
  var OK = google.maps.places.PlacesServiceStatus.OK;
  var OVER_LIMIT = 'OVER_QUERY_LIMIT';

  return new Promise(function(resolve, reject){
    service.textSearch({ query: query }, function (results, status) {
      var loc = results[0].geometry.location;

      if(status === OK ){
        resolve({ lat: loc.lat(), lang: loc.lang() });
      } else if(status === OVER_LIMIT){

        // The most interesting part. Reschedule a retry.
        setTimeout(function(){

          // When the promise of this call resolves then...
          // call the resolve/reject of the enclosing (previous) promise
          getLocation(query, map, retryTimeout).then(resolve, reject);
        }, retryTimeout);

      } else {
        reject(new Error(status));
      }
    });
  });
}

// Usage:
var object= {...}
var locationPromise = getLocation(query, map, 1000);

locationPromise.then(function(locationData){
  // Only then do you mutate. Only have getLocation get the location data.
  // Mutating object is not its concern. It's the consumer's concern.
  object.lat = locationData.lat;
  object.lang = locationData.lang;
});

Now here's the trick. In order to make getLocation act like a recursive call, you simply call getLocation again, but hand over the resolve and reject of the enclosing promise as its callbacks. When the succeeding getLocation call succeeds, it will call the previous promise's callbacks.

So in terms of execution, it's not really recursive and the entire operation is still async. However, how the promises enclose each other one call after the other and pass on their resolve and reject as callbacks to the next makes it act recursive.

Of course we cannot escape the wrath of setTimeout since we need it for delay.

Since Promises are not widely supported yet, you can use Angular's built-in $q which does the same thing, and probably more.