0

I've written a pretty simple test app as follows:

angular.module('tddApp', [])

.controller('MainCtrl', function ($scope, $rootScope, BetslipService) {
    $scope.displayEvents = [
        {
            id: 1,
            name: 'Belarus v Ukraine',
            homeTeam: 'Belarus',
            awayTeam: 'Ukraine',
            markets: {home: '2/1', draw: '3/2', away: '5/3'},
            display: true
        }
    ];
    $scope.betslipArray = BetslipService.betslipArray;
    $scope.oddsBtnCallback = BetslipService.addToBetslip;
    $scope.clearBetslip = BetslipService.clearBetslip;
})

.directive('oddsButton', function () {
    return {
        template: '<div class="odds-btn">{{market}}</div>',
        replace: true,
        scope: {
            market: '@',
            marketName: '@',
            eventName: '@',
            callback: '&'
        },
        link: function (scope, element) {

            element.on('click', function() {
                scope.callback({
                    name: scope.eventName,
                    marketName: scope.marketName,
                    odds:scope.market
                });
            });
        }
    };
})

.factory ('BetslipService', function ($rootScope) {
    var rtnObject = {};

    rtnObject.betslipArray = [];

    rtnObject.addToBetslip = function (name, marketName, odds) {
        rtnObject.betslipArray.push({
            eventName: name,
            marketName: marketName,
            odds: odds
        });
    };

    rtnObject.clearBetslip = function () {
        rtnObject.betslipArray = [];
    };

    return rtnObject;
});

I've assigned an array to a controller variable. I've also assigned functions to modify the array. To add an object to the array the callback is called by a directive with isolate scope. There's some strange behaviour happening that I don't quite understand:

=> clicking the directive runs the callback in the service. I've done some debugging and it seems that the controller variable is updated but it doesn't show in the html. => clicking the button to clear the array isn't working as expected. The first time it's causing an element to display, after which it has no effect.

I think that this may have to do with the nested ng-repeats creating their own scopes

NB

I fixed the array not clearing by changing the function in the service to:

while (rtnObject.betslipArray.length > 0) {
    rtnObject.betslipArray.pop();
}
// instead of 
rtnObject.betslipArray = [];

This makes sense as the service variable was being pointed at a new object while the old reference would persist in the controller.

I got the html to update by wrapping the callback call in the directive in a scope.$apply(). This part I dont really understand. How can scope.$apply() called in the directive have an effect on the controller scope when the directive has an isolate scope? updated fiddle: http://jsfiddle.net/b6ww0rx8/7/

Any thought's greatly appreciated

jsfiddle: http://jsfiddle.net/b6ww0rx8/5/

C

Cathal
  • 1,740
  • 18
  • 31
  • I actually add some `console.log` statements in your service and a watch statement in your controller for the array, and i found that your controller array is not being updated. The array in the service is but not the controller. – m.e.conroy Oct 10 '14 at 13:40
  • I did a similar test. I'm not sure why the $watch isn't picking this up but if I assign to the controller scope to a variable within the service and log it as per http://jsfiddle.net/b6ww0rx8/6/ it looks like the controller is being updated – Cathal Oct 10 '14 at 13:45
  • You're adding a resource and time consuming while loop in order to truncate your array? On top of that you're using `pop()` on your array in order to remove items entirely, which also has a cost? `array.length = 0`, one line no loop, no time suck. What happens if you happen to have thousands of items in that array? Your while loop may take too long and hang your application. – m.e.conroy Oct 10 '14 at 14:35

1 Answers1

1

I got it working here: http://jsfiddle.net/b6ww0rx8/8/

Added $q, $scope.$emit and $timeout clauses to help with communications between your directive / service and controller.

I would like to also say that I wouldn't assign service functions to a controller $scope, You should define functions in the controller that call service functions.

Instead of this:

$scope.clearBetslip = BetslipService.clearBetslip;

Do this:

$scope.clearBetslip = function(){
    BetslipService.clearBetslip().then(function(){
        $scope.betslipArray = BetslipService.getBetslipArray();
    });
};
m.e.conroy
  • 3,508
  • 25
  • 27
  • I'm not sure if I agree with you on this. SUrely the angular way is to abstract as much logic as possible away from the controllers. What's wrong with assigning a service method to a $scope variable? – Cathal Oct 10 '14 at 14:02
  • 1
    Well for one, the example on this page. You run into problems where you think you have a reference to the array and while it updates in the service it doesn't update in the controller. In turn assigning the `clear` function in this example directly to controller scope doesn't allow the controller to update the array after clear executes, something you don't want to do inside the service because the service shouldn't be concerned with the controller's variable. – m.e.conroy Oct 10 '14 at 14:06
  • 1
    Services should retrieve, save / send, manipulate data and house the logic for that, in this way yes you are correct, the service is abstracting the logic away from the controller. But the controller should know how to use that service, in no way does my example above add extra logic to the controller. The controller only calls a service method to maniuplate the data and waits on the service to finish, if its successful then update the controller's array such that we can update the display accordingly. There's very minimal logic involved in the promise and in this case its appropriate. – m.e.conroy Oct 10 '14 at 14:12
  • This isn't caused by referencing the service variable in the controller. The clear wasn't working as I was assigining a new array to the variable in the service. This meant the controller and service were no longer pointing at the same object. The issue with adding an item only occurs when the callback is called from the directive scope, wrapping this call in a scope.$apply solves – Cathal Oct 10 '14 at 14:14
  • There in lies the problem. If you were to use someone else's service, for argument sake, that did the same thing you were doing here, upon clear assigning the internal service's array to a brand new array your controller no longer **references** the array you need from the service. Thus its better that the service's array is private like in a class object and you call a getter to enact changes in your controller's array. – m.e.conroy Oct 10 '14 at 14:31
  • I take your point with the array assignment on clear. It's better to hide the array and use the promise method as you suggest but I think that triggering the digest from within the directive is better when adding the element then using timeout and emit. – Cathal Oct 10 '14 at 14:43
  • Yes, there probably is a better way than using timeout and emit. – m.e.conroy Oct 10 '14 at 14:48
  • btw as per this post http://stackoverflow.com/questions/1232040/how-to-empty-an-array-in-javascript using array.pop in a while loop is significantly faster than setting array.length = 0 – Cathal Oct 10 '14 at 14:51