AngularJS: When to pass $scope variable to function

Question

I am using the TodoMVC app to get better with the AngularJS framework. In the index.html on lines 14-16 you see this:

<form id="todo-form" ng-submit="addTodo()">
    <input id="new-todo" placeholder="What needs to be done?" ng-model="newTodo" autofocus>
</form>

Notice how the ng-submit directive calls the addTodo() function without the newTodo model being passed as an argument.

A short time later I came across the following code in the very same file on line 19:

<input id="toggle-all" type="checkbox" ng-model="allChecked" ng-click="markAll(allChecked)">

You can see the author decided to pass the allChecked model to the markAll() function this time. If I understand correctly, they could have referenced $scope.allChecked inside the controller instead of passing it in.

Why use two different approaches in the same file? Is one approach better in some circumstances? Is this a case of inconsistency or is there a deeper logic being used?

Answer 1

I would prefer to always pass in arguments to the function:

• It's clearer what parameters the function expects.
• It's easier to unit-test because all parameters are injected into the function.(good for unit- testing)

Consider the following situation:

$scope.addToDo = function(){
   //This declaration is not clear what parameters the function expects.
   if ($scope.parameter1){
      //do something with parameter2
   }    
}

And even worse:

$scope.addToDo = function(){
    //This declaration is not clear what parameters the function expects.
    if ($scope.someobject.parameter1){ //worse

    }    
}

Because of scope inheritance parameter2 may come from parent scope, accessing parameter2 inside the function creates a tight coupling, also causes troubles when you try to unit-test that function.

If I define the function like this:

//It's clearer that the function expects parameter1, parameter2
$scope.addToDo = function(parameter1, parameter2){
   if (parameter1){
      //do something with parameter2
   }    
}

In case your parameter2 is inherited from parent scope, you could still pass it in from the view. When you do unit-testing, it's easy to pass all parameters.

If you have ever worked with ASP.NET MVC, you would notice something similar: the framework tries to inject parameters into action function instead of accessing it directly from Request or HttpContext object

It's also good in case others have mentioned like working with ng-repeat

In my opinion, Controller and Model in angular are not quite clearly separated. The $scope object looks like our Model with properties and methods (Model contains also logic). People from OOP background would think that: we only pass in parameters that don't belong to object. Like a class Person already has hands, we don't need to pass in hands for every object method. An example code like this:

//assume that parameter1 belongs to $scope, parameter2 is inherited from parent scope.
    $scope.addToDo = function(parameter2){ 
        if ($scope.parameter1){ //parameter1 could be accessed directly as it belongs to object, parameter2 should be passed in as parameter.
            //do something with parameter2
        }   
    }

Answer 2

There are two parts to this answer, the first part is answering which one is the better option, the other part is the fact that neither of them is a good option!

---

Which one is correct?

This one is:

$scope.addToDo = function(params1, ...) {
    alert(params1);
}

Why? Because A - it is testable. This is important even if you are not writing tests, because code that is testable is pretty much always more readable and maintainable in the long run.

It is also better because of B - it is agnostic when it comes to the caller. This function can be reused by any number of different controllers/services/etc because it does not depend on the existence of a scope or on the structure of that scope.

When you instead do this:

$scope.addToDo = function() {
    alert($scope.params1);
}

Both A and B fail. It is not easily testable on its own, and it cannot easily be reused because the scope you use it in might be formatted differently.

Edit: If you are doing something very closely tied to your specific scope and running the function from the template, then you might run in to situations where trying to make it reusable just doesn't make sense. The function simply isn't generic. In that case, don't bother with that, some functions cannot be reused. View what I wrote about as your default mode but remember that in some cases it won't fit.

---

Why are both wrong?

Because as a general rule you should not be doing logic in your controllers, that is the job of a service. The controller can use a service and call the function or expose it in a model, but it should not define it.

Why is this important? Because again it makes it easy to reuse the function. A function that is defined in a controller cannot be reused in another controller without putting limits on how the controllers are invoked in the HTML. A function that is defined in a service can be injected and reused wherever you feel like it.

But I don't need to reuse the function! - Yes you do! Maybe not right now and maybe never for this specific function, but sooner or later you will end up wanting to reuse a function that you where convinced that you would never need to reuse. And then you will have to rework code that you have already half forgotten, which always take extra time.

It's better to just do it properly from the start and move all logic that you can into services. That way, if you ever need them somewhere else (even in another project) you can just grab it and use it without having to rewrite it to fit your current scope structure.

Of course, services don't know about your scope so you are forced to use the first version. Bonus! And don't fall for the temptation of passing the entire scope to a service, that will never end well :-)

So this is IMO the best option:

app.service('ToDoService', [function(){
    this.addToDo = function(params1, ...){
        alert(params1);
    }
}]);

And inside the controller:

$scope.addToDo = ToDoService.addToDo;

---

Note that I wrote "general rule". In some cases it is reasonable to define the function in the controller itself as opposed to a service. One example would be when the function only relates to scope specific things, like toggling a state in the controller somehow. There is no real way to do that in a service without things becoming strange.

But it sounds like this is not the case here.

Answer 3

The Zen of Angular suggests:

Treat scope as read only in templates
Treat scope as write only in controllers

Following this principle, you should always call functions explicity with parameters from the template.

However, in any style you follow, you do have to be careful about priorities and order of execution of directives. In your example, using ng-model and ng-click leaves the order of execution of the two directives ambiguous. The solution is using ng-change, where the order of execution is clear: it will be executed only after the value changes.

Answer 4

The custom behavior methods, such as ng-click, ng-submit etc. allow us to pass parameters to methods being called. This is important since we may want to pass something that may not be available freely across till the handler in the controller. For eg,

angular.module('TestApp')
.controller('TestAppController', ['$scope', function($scope) {
    $scope.handler = function(idx) {
        alert('clicked ' + idx.toString());
    };
}]);

<ul>
    <li ng-repeat="item in items">
        <button ng-click="handler($index)">{ item }</button>
        <!-- $index is an iterator automatically available with ngRepeat -->
    </li>
</ul>

In case of your second example, since allChecked is within the scope of the same controller which defines markAll(), you are absolutely correct, there is no need to pass anything. We are only creating another copy.

The method would have to simply be refactored to use whats available in the scope.

$scope.markAll = function () {
    todos.forEach(function (todo) {
        todo.completed = $scope.allChecked;
    });
};

Hence, even though we have the option to use parameters in these methods, they are only required some of the time.

Answer 5

I think that this is simply a case of inconsistency in the code. I've pondered this question before and came to the following conclusion...

Rule: Don't pass $scope variables into $scope functions.

Reading the controller code should be enough code to demonstrate what the function of the component will be. The view should not contain any business logic, just bindings (ng-model, ng-click etc). if something in the view can be made clearer by being moved to the controller, so be it.

Exception: Allow conditional ng-class statements (e.g. ng-class='{active:$index==item.idx') - putting class conditionals in the controller can be very verbose, and muddies the logic of the controller with ideas from the view. If it's a visual property, keep it in the view.

Exception: You are working with an item in an ng-repeat. For example:

<ul ng-repeat="item in items">
    <li><a ng-click="action(item)"><h1>{{item.heading}}</h1></a></li>
</ul>

I follow these rules when writing controllers & views, and they seem to work. Hope this helps.

Answer 6

Perhaps to illustrate that you can? There is no functional difference between the two assuming that they are the same controller. Note that there are situations where child scopes are generated in which case you won't have the same scope as the controller any longer.