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?
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);
});
}