Search code examples
angularjsangularjs-factory

Refactoring - Checking if promise has been resolved before


This function get an user files list provided by an external API. Once that the data has been retrieved it is saved, so the next time it is called it returns the same data previously saved. This code is at controller level.

var userFiles = {};

function getUserFiles(user) {
  var dfd = $q.defer();

  // Files already have been loaded
  if (userFiles[user.id]) {
    dfd.resolve(userFiles[user.id]);
  } else {

    // Get files for the first time
    Users.getFiles(user).then(function(files) {
      userFiles[user.id] = files;
      dfd.resolve(userFiles[user.id]);
    });
  }

  return dfd.promise;
}

How would you refactor this code so it calls dfd.resolve only once?


Solution

  • EDIT #2

    I've got an even better snippet for you:

    function getUserFiles(user) {
      return userFiles[user.id] || (userFiles[user.id] = Users.getFiles(user));
    }
    

    This sets userFiles[user.id] to be the promise returned by Users.getFiles(user) and returns it. If userFiles[user.id] has already been set (i.e. it is truth-y) then it just returns the previously resolved promise.

    The disadvatange using this super short snippet is that you aren't doing any failure checking - and could end up caching a failed request - then again, you weren't doing any failure checking in your other code either.

    EDIT

    According to the docs, you can call $q.resolve() directly without needing to create the intermediate deferred object - i.e. $q.defer().resolve()

    A minor refactor, the following code saves you from creating a deferred when you don't need to. You only need to create a new deferred if the files have already been loaded. Otherwise, you can just return the call to Users.getFiles(), like this:

    function getUserFiles(user) {
    
      // Files already have been loaded
      if (userFiles[user.id]) {
    
    //        See Edit Note
    //        return $q.defer().resolve(userFiles[user.id]);
            return $q.resolve(userFiles[user.id]);
      }
    
      // Get files for the first time
      return Users.getFiles(user)
        .then(function(files) {
          //Cache them
          return (userFiles[user.id] = files);
        });
    
    }