3

I'm trying to get the response of a $http call from ng-repeat inputs. And then determining whether to show the individual ng-repeat's given each response. Currently the actual $http api isn't available, so I've just hardcoded the response.

It works just fine without using promises. But I am unable to even run it on jsfiddle when using the promise method. How can I get the promise working?

EDIT: I've reworked my question thanks to Benjamin Gruenbaum I'm not actually using $http but using pouchDB (local DB so no need to worry about DOS). Just using $http as an example since it returns a promise as well. What I really need to work out is how to get the promises to update again when adding new names. e.g If "joe" is added then he should be on 'billsNotPaid'

http://jsfiddle.net/TheSlamminSalmon/cs0gxxxd/6/

View:

<div ng-app>    
    <div ng-controller="MainController">        
        <h1>Hello Plunker!</h1>
        <form ng-submit="AddNewGuy()">
            <input type="text" ng-model="newGuy" placeholder="Insert New Guy"></input>
            <input type="submit"></input>
        </form>

        <strong>Everyone</strong>
        <div ng-repeat="name in names">
            {{name}}
        </div>

        <strong>These guys haven't paid their bills (Non Promise)</strong>
        <div ng-repeat="name in names">
            <div ng-show="NotPaidBillsNonPromise(name)">
                {{name}}
            </div>
        </div> 

        <strong>These guys haven't paid their bills (Using http Promise)</strong>
        <div ng-repeat="name in billsNotPaid">
                {{name}}
        </div>
    </div>
</div>

AngularJS Controller:

function MainController($scope, $http) {
    $scope.names = [
        "James",
        "Tim",
        "Alex",
        "Sam",
        "Kim"
    ];
    $scope.billsNotPaid = []; // start as empty

    $scope.NotPaidBillsNonPromise = function (name) {
        if (name == "Tim" || name == "Sam" || name == "Joe") return true;
    };

    $scope.NotPaidBills = function (name) {
        return $http.get("http://echo.jsontest.com/name/" + name)
        .then(function (r) {
                return (r.data.name === "Tim" || r.data.name === "Sam" || r.data.name === "Joe")
        });
    };

    $scope.newGuy;
    $scope.AddNewGuy = function () {
        $scope.names.push($scope.newGuy);
    };

    // start the check for each name
    $scope.names.forEach(function(name){ 
        return $scope.NotPaidBills(name).then(function(notPaid){
            console.log(name, notPaid);
            if(notPaid) $scope.billsNotPaid.push(name); 
        });
    });
}
Jimmy To
  • 161
  • 3
  • 10
  • The `NotPaidBills` function returns `undefined`. – Sacho Apr 04 '15 at 10:24
  • 1
    As a general note - I strongly suggest you avoid making n requests for n inputs - network traffic is expensive. – Benjamin Gruenbaum Apr 04 '15 at 10:42
  • As Benjamin notes, what your doing here is really bad, but it's even worse than what he says... Your implementation as it is here, will for N names, make N server requests PER digest cycle, every time angular runs one... Have a good amount of clients running and you sort of just implemented a DoS attack against your self o.O... (As a side note, ng-if is often preferable to ng-show/ng-hide) – Jens Apr 04 '15 at 11:09

2 Answers2

3

First thing first - I strongly suggest you avoid making an HTTP request per item in an ng-repeat. It's slow and will likely cause bad user experience - instead I suggest you batch these requests into a single request that takes multiple values and returns multiple values.


Angular performs updates via two way data binding, the binding values update through a loop called the digest cycle - so whenever Angular runs such a cycle all your values are guaranteed to be up to date to the time the cycle is run.

Luckily for you - since $http returns a promise - Angular will schedule a digest after the call ends since promises run their then callbacks via $evalAsync which means a digest will be scheduled if one is not already scheduled or in progress.

So you can just update the scope from the $http promise fulfilment handler and it'll work. We add a new scope property:

$scope.billsNotPaid = [];

Here's the corresponding Angular:

<div ng-repeat="name in billsNotPaid">
    {{name}}
</div>

And the call:

$scope.NotPaidBills = function (name) {
    return $http.get("http://echo.jsontest.com/name/" + name)
    .then(function (r) {
            return (r.data.name === "Tim" || r.data.name === "Sam")
    });
};

// start the check for each name
$scope.names.forEach(function(name){ 
    return $scope.NotPaidBills(name).then(function(notPaid){
        console.log(name, notPaid);
        if(notPaid) $scope.billsNotPaid.push(name); 
    });
});

Fiddle

Benjamin Gruenbaum
  • 270,886
  • 87
  • 504
  • 504
  • Thanks for this information. Actually I won't be using the $http to obtain the actual result. I actually using pouchDB which returns promises as well. Just using $http as a work around to understand promises. I have updated my question since I need the "names" list to be dynamic. Things can be added onto it. So the initial foreach loop will only one once. – Jimmy To Apr 04 '15 at 11:05
  • @JimmyTo in that case you need to make sure that the promises that are returned indeed schedule a digest - most promise code as code from outside angular will not do it for you - you can wrap the promise with `$q.when` and only then `.then` it in order to convert it from a generic promise (thenable) to a trusted Angular promise or set the scheduler. Alternatively you can use an angular wrapper for pouchdb like https://github.com/wspringer/angular-pouchdb which basically does this for you- from what I can tell that one in particular even has an ad-hoc solution for your specific ng-repeat issue – Benjamin Gruenbaum Apr 04 '15 at 11:08
1

Nothing new, just summarizing what have been said in other answers.

  1. Don't make chatty interface if you are crossing network boundary, especially when it's inside a loop like ng-repeat. Therefore, you should load whatever necessary to render your view ahead of time (i.e. when the view get activated) and for each new item you need to add to the view, you add it directly to the UI in the add handler and use a service to persist it back to the data store (which most likely will be a asynchronous promise based call if going across network boundary / the api is promise based).

  2. Updated per Benjamin's comment... Alternatively to $http when using jsfiddle or something similar, you can definitely simulate going across network boundary by using angular $q or $timeout service. In short, create a defer object by calling var dfd = $q.defer();, resolve the promise using dfd.resolve and finally return the promise using return dfd.promise or wrap the simulated network call using $timeout and just return the result inside the timeout function like so:

--

function asyncSomething() {
  var dfd = $q.defer();
  var result = 1 + 1; //do something async
  dfd.resolve(result);
  return dfd.promise;
}

or

function asyncSomething() {
    return $timeout(function() {
        return 1 + 1; ///do something async
    }, 1000);  //pretend async call that last 1 second...
}
  1. And yes, do use John Papa's Angular style guide if you are not following any yet. It's just good practice.

Here is your fiddle altered in John Papa's style as an example of what mentioned above: http://jsfiddle.net/kx4gvsc0/30/

In the example above, I am trying to solve the problem in a different way. Instead of looping through all the names in the list, I am handing over the responsibility of refreshing the owing list to the addCustomer method (Yeah, not a good idea, you can separate this into to 2 calls that is more SRP compliant but for the sake of making the call chunky...), in any case, as part of the async add customer call, it chain another promise (that might be the pouchdb call which is also promise base) that will refresh the owing list and return that back to the controller for binding.

    function addCustomerAndRefreshOwingList(customerName) {
        return $timeout(function() {                
            //persist the new customer to the data source
            _customers.push({ name: customerName, paid: false});
        }, 250) //pretend network request lasting 1/4 second
        .then(refreshOwingList);  //chain another promise to get the owing list <-- This is your pouch db call perhaps...
    }

    function refreshOwingList() {
        return $timeout(function() {                                
            //Fake logic here to update certain payers per call using the counter
            switch(counter) {
                case 0:  // first pass...
                    _customers[1].paid = true; //Tim paid
                    break;
                case 1:  // second pass...
                    _customers[2].paid = true;  //Alex paid
                    _customers[4].paid = true;  //Kim paid
                    break;
                default:
                    break;
            }

            counter++;

            //return all customers that owes something
            return _customers
                .filter(function(c) { return !c.paid; })
                .map(function(c) { return c.name; });

        }, 500); //pretend this is taking another 1/2 second        
    }
Jimmy Chandra
  • 6,472
  • 4
  • 26
  • 38
  • Point #2 is [the deffered anti-pattern](http://stackoverflow.com/questions/23803743/) since $timeout already returns a promise. Point #3 is just irrelevant to the question and is opinion based. Point #1 is just good advice. I fail to see how you're solving OP's problem. – Benjamin Gruenbaum Apr 04 '15 at 14:02
  • Ah didn't realize that... I need to give it a try without the deferred. – Jimmy Chandra Apr 05 '15 at 12:53
  • After reading your comment and re-reading the question again, I've corrected the example and my answer above. – Jimmy Chandra Apr 05 '15 at 13:56