1

I'm trying to wrap by head around when to use $q in AngularJS. I've been using it in all my services so far so when I call our Web API it works nicely. What I'm trying to do is cut down on all the calls to the API since I need the same data in multiple places and I've just been pinging the API every time I needed the data.

Right now I have a service, which gets the data and a helper file to help with related things about the data.

What I want is to use this helper factory to hold the data that's needed for every body which uses it.

I'm having trouble wrapping my head around assigning the value of data from the helper file if the data hasn't gotten back to me yet when AngularJS runs.

This is what I have so far.

(function() {
  var app = angular.module("Test", []);
  app.service("githubService", function($http, $q) {
    var deferred = $q.defer();

    this.getAccount = function() {
      return $http.get('https://api.github.com/users/JonDoe');
    };
  });

  app.factory("githubHelper", ["githubService", function(githubService) {
    _gitHubInfo = {};

    githubService.getAccount().then(function(data) {
      _gitHubInfo = data;
    });

    return {
      gitHubInfo: _gitHubInfo
    };
  }]);

  app.controller("Dummy", ["$scope", "githubHelper", "githubService", function($scope, githubHelper, githubService) {
    
    // How do I make it work this way?
    $scope.value = githubHelper.gitHubInfo;
    
    // This is what I'm using now
    githubService.getAccount().then(function(data) {
      $scope.value2 = data;
    });
  }]);
})();
.box {
  border: 1px red solid;
}
<script src="https://cdnjs.cloudflare.com/ajax/libs/angular.js/1.7.5/angular.min.js"></script>
<div ng-app="Test">
  <div ng-controller="Dummy">
    <div class="box">
      {{value}}
    </div>
    <br />
    <div class="box">
      {{value2}}
    </div>
  </div>
</div>

What I want to do is just remove the githubService dependency from the Dummy controller and only have it present in the githubHelper factory so I can remove the dependency on githubService from all my controllers and instead use gitHubHelper.

What do I need to change in Dummy controller to have $scope.value be the data returned from githubService.getAccount()?

Jimenemex
  • 3,104
  • 3
  • 24
  • 56
  • 1
    Avoid the [deferred Anti-Pattern](https://stackoverflow.com/questions/30750207/is-this-a-deferred-antipattern). – georgeawg Feb 19 '19 at 03:24
  • In fact, you're doubly-antipatterning because you return the $http promise, and then *returning the deferred promise from the resolver*... it would have sufficed to just *call* http.get, then return deferred.promise immediately after. Antipattern, yes, but less head scratchy than this – Leroy Stav Feb 19 '19 at 03:35
  • Updated to now just return the promise from `$http.get`. – Jimenemex Feb 19 '19 at 03:37

2 Answers2

3

I have something like this in my project:

app.factory("githubHelper", ["githubService", function(githubService) {
    var promise = null;

    function getInfo() {
      if (!promise) {
        promise = githubService.getAccount();
      }
      return promise;
    }

    return {
      getInfo: getInfo
    };
  }]);

githubHelper.getInfo().then(function(data) {})
githubHelper.getInfo().then(function(data) {})
githubHelper.getInfo().then(function(data) {})
...
Petr Averyanov
  • 9,327
  • 3
  • 20
  • 38
  • Why don't you just `return githubService.getAccount();`. Why save the promise object at all? – Jimenemex Feb 19 '19 at 15:25
  • @Jimenemex Because the OP wants to cache it and not make another request. This is the correct solution. – Bergi Feb 19 '19 at 19:34
  • @Bergi Sorry, but what's the different if I just remove the function `getInfo()` and expose the service function in the helper `return { getInfo: githubService.getAccount}`. Then I could just invoke on the function and have the `.then` in the controller? `githubHelper.getInfo().then(...` I wouldn't need to have `var promise` then. – Jimenemex Feb 20 '19 at 18:25
  • @Jimenemex The usage is exactly the same as with the code in this answer. The difference is that multiple invocations of `getInfo` all lead to the same result, with only a single request being made (from the first call that sets the `promise`). – Bergi Feb 20 '19 at 20:41
  • @Jimenemex Oh, i just realised you are the OP. Is this not what you want? – Bergi Feb 20 '19 at 20:42
  • @Bergi The result yes, but I wanted to understand. – Jimenemex Feb 20 '19 at 20:48
-1

You're almost there. The problem is that githubinfo doesn't get populated until after you access it. What you should do is almost exactly (see my comment above) what you're doing githubservice.getaccount, but in githubhelper.getaccount, instead. Set githubinfo to a $q.deferred, return githubinfo.promise from getaccount, and resolve the promise in the then

UPDATE: NOW WITH MORE CODE! (now that I'm not on mobile :-D)

(function() {
  var app = angular.module("Test", []);
  app.service("githubService", function($http, $q) {
    this.getAccount = function() {
      return $http.get('https://api.github.com/users/JonDoe');
    };
  });

  app.factory("githubHelper", ["githubService", function(githubService) {
    return {
      gitHubInfo: $q(function(resolve, reject) {
        githubService.getAccount().then(function(data) {
          resolve(data);
        }, reject);
      }
    };
  }]);

  app.controller("Dummy", ["$scope", "githubHelper",
    function($scope, githubHelper) {

      githubHelper.gitHubInfo.then(function(data) {
        $scope.value = data;
      });

  }]);
})();

Now, written as-is I would never approve that as a PR for many reasons (code clarity re: return { someProp: function() { /* a bunch of code */} } .... usage of $scope should be avoided at all costs... and as mentioned, the $cacheFactory can and should handle this) , but you should be able to get the general gist of how something along these lines could work

Leroy Stav
  • 722
  • 4
  • 12
  • 1
    Wouldn't I need to surround the call to `githubService.getAccount()` inside of it's own method in the service and return the `promise` from inside there, while exposing the new method in the `return {}`? Wouldn't I also need to have the `.then` callback in the controller? – Jimenemex Feb 19 '19 at 04:01
  • Sorry, slightly misread your code. You set the `githubinfo` property of the return to `githubinfo.promise` and resolve it in the `then` of `getAccount`. You should also consider setting the reject callback to `githubinfo.reject`, but that's another story. Yes, you'll have to have a `.then in your controller, but that's the nature of sync development. The only other option you would have is to initiate the request in a route resolver but I don't think you want that. Question: why not use the cacheFactory? – Leroy Stav Feb 19 '19 at 04:38
  • Avoid the [`Promise` constructor antipattern](https://stackoverflow.com/q/23803743/1048572?What-is-the-promise-construction-antipattern-and-how-to-avoid-it)! – Bergi Feb 19 '19 at 19:33
  • @Bergi Yes, "avoid" but not "never use"... given certain circumstances it is a completely valid approach (tho not necessarily in this case) and if you notice, I did it "correctly" in my updated code example :-) Very specifically, it is valid if he wants to generate a new promise after some `n` times that the data is requested (or after some `t` time) and needs to replace `_gitHubInfo` with a new promise to be resolved. Yes, there are ways to avoid it there, as well, but IMO those tend to be anti-pattern anti-patterns that should be avoided entirely – Leroy Stav Feb 19 '19 at 20:31
  • No, it's really a "never use" pattern, and definitely in this case as well. `$q(function(resolve, reject) { githubService.getAccount().then(resolve, reject);` should absolutely be avoided. You don't need `$q(…)` to replace a promise with a different one either. – Bergi Feb 19 '19 at 20:41
  • Use `$q.when` when you are dealing with an object that might or might not be a promise, or if the promise comes from a source that can't be trusted. Don't use the promise constructor. It is unnecessary and novice programmers are prone to using it erroneously. – georgeawg Feb 21 '19 at 18:56