Search code examples
javascriptangularjsfunctioncoding-stylehoisting

Are variable bound / 1st class functions preferable over private method naming? How is hoisting affected?


A few questions regarding structuring Angular code and the behavior of JavaScript when using variable bound vs private method naming function conventions. Is there a performance or stylistic reason for using variable bound functions / first class functions in AngularJS over private method naming? How is hoisting affected in each method? Would the second method below reduce the amount of hoisting performed and would this have a noticeable affect on application performance?

An example of private method naming. Is this a recommended way to structure Angular code?

    (function () {
       'use strict'
        function modalController(dataItemsService, $scope) {
            var vm = this;

            function _getSelectedItems() {
               return dataItemsService.SelectedItems();
            }

            function _deleteSelectedItems() {
               dataItemService.DeleteItem();
               $("#existConfirmDialog").modal('hide');
            }

            vm.SelectedItems = _getSelectedItems;
            vm.deleteItemRecord = _deleteItemRecord;
        }

        angular.module('app').controller('modalController', ['dataItemService', '$scope', modalController]
})();

An example of variable bound functions. What about this method of structuring angular code within the controller - is there any disadvantage or advantage in terms of performance/style?

angular.module('appname').controller("NameCtrl", ["$scope", "$log", "$window", "$http", "$timeout", "SomeService",
    function ($scope, $log, $window, $http, $timeout, TabService) {

    //your controller code
       $scope.tab = 0;    
       $scope.changeTab = function(newTab){
          $scope.tab = newTab;
       };    
       $scope.isActiveTab = function(tab){
          return $scope.tab === tab;
       }; 
    }
]);

Solution

  • The first method, using "private" methods and exposing them via public aliases, is referred to as the Revealing Module Pattern, although in the example the methods aren't actually private.

    The latter is a pretty standard Constructor Pattern, using $scope as context.

    Is there a performance or stylistic reason for using variable bound functions / first class functions in AngularJS over private method naming?

    Is [there] a recommended way to structure Angular code?

    TL;DR

    • Fundamentally, there isn't much difference between the two styles above. One uses $scope, the other this. One Constructor function is defined in a closure, one is defined inline.

    • There are scenarios where you may want a private method or value. There are also stylistic and (probably insignificant) performance reasons for using the variable this/vm over $scope. These are not mutually exclusive.

    • You'll probably want to use a basic, bare bones, old school Constructor Pattern, and a lot of people are exposing state and behavior via this instead of $scope.

    • You can allow yourself data privacy in the Controller, but most of the time this should be leveraged by a Service/Factory. The main exception is data representative of the state of the View.

    • Don't use jQuery in your Controller, please.

    References:

    AngularJS Style Guide by Todd Motto.

    AngularJS Up & Running


    To answer your question thoroughly, I think it important to understand the responsibility of the Controller. Every controller's job is to expose a strict set of state and behavior to a View. Put simply, only assign to this or $scope the things you don't mind your user seeing or playing with in your View.

    The variable in question (vm, $scope) is the context (this) of the instance being created by the Controller function.

    $scope is Angular's "special" context; it has some behaviors already defined on it for you to use (e.g. $scope.$watch). $scopes also follow an inheritance chain, i.e. a $scope inherits the state and behaviors assigned to its parent $scope.

    Take these two controllers:

    angular.module("Module")
        .controller("Controller", ["$scope", function($scope) {
            $scope.tab = 0;
            $scope.incrementTab = function() {
                $scope.tab++;
            };
        }])
        .controller("OtherController", ["$scope", function($scope) {
            // nothing
        }]);
    

    And a view

    <div ng-controller="Controller">
        <p>{{ tab }}</p>
        <button ng-click="incrementTab();">Increment</button>
        <div ng-controller="OtherController">
            <p>{{ tab }}</p>
            <button ng-click="incrementTab();">Increment</button>
        </div>
    </div>
    

    Example here

    What you'll notice is that even though we didn't define $scope.tab in OtherController, it inherits it from Controller because Controller is it's parent in the DOM. In both places where tab is displayed, you should see "0". This may be the "hoisting" you're referring to, although that is an entirely different concept.

    What's going to happen when you click on the first button? In both places we've exposed "tab", they will now display "1". Both will also update and increment when you press the second button.

    Of course, I may very well not want my child tab to be the same tab value as the parent. If you change OtherController to this:

    .controller("OtherController", ["$scope", function($scope) {
        $scope.tab = 42;
    }]);
    

    You'll notice that this behavior has changed - the values for tab are no longer in sync.

    But now it's confusing: I have two things called "tab" that aren't the same. Someone else may write some code later down the line using "tab" and break my code inadvertently.

    We used to resolve this by using a namespace on the $scope, e.g. $scope.vm and assign everything to the namespace: $scope.vm.tab = 0;

    <div ng-controller="OtherController">
        <p>{{ vm.tab }}</p>
        <button ng-click="vm.incrementTab();">Increment</button>
    </div>
    

    Another approach is to use the simplicity and brevity of this and take advantage of the controllerAs syntax.

    .controller("OtherController", function() {
        this.tab = 0;
    });
    
    <div ng-controller="OtherController as oc">
        <p>{{ oc.tab }}</p>
    </div>
    

    This may be more comfortable for people who are used to using plain JS, and it's also easier to avoid conflicts with other Angular sources this way. You can always change the namespace on the fly. It's also a bit "lighter" on performance since you're not creating a new $scope instance, but I'm not sure there's much gain.


    In order to achieve privacy, I would recommend encapsulating your data in a Service or Factory. Remember, Controllers aren't always singletons; there is a 1:1 relationship between a View and a Controller and you may instantiate the same controller more than once! Factories and Service objects are, however, singletons. They're really good at storing shared data.

    Let all Controllers get a copy of the state from the singleton, and make sure all Controllers are modifying the singleton state using behaviors defined on the Service/Factory.

    function modalController(dataItemsService) {
        var vm = this;
        vm.selectedItems = dataItemsService.SelectedItems(); // get a copy of my data
        vm.updateItem = dataItemService.UpdateItem; // update the source
    }
    

    But wait, how do I know when another part of my app has changed my private data? How do I know when to get a new copy of SelectedItems? This is where $scope.$watch comes into play:

    function modalController(dataItemsService, $scope) {
        var vm = this;
    
        vm.updateItem = dataItemService.UpdateItem; // update the source
    
        // periodically check the selectedItems and get a fresh copy.
        $scope.$watch(dataItemsService.SelectedItems, function(items) {
            vm.items = items;
        });
    
        // thanks $scope!
    }
    

    If your data is not shared, or if your private data is representative of the View layer and not the Model layer, then it's totally OK to keep that in the controller.

    function Controller() {
        var buttonClicked = false;
    
        this.click = function() {
            buttonClicked = true; // User can not lie and say they didn't.
        };
    }
    

    Lastly, DO NOT USE JQUERY IN YOUR CONTROLLER, as your reference did!

    $("#existConfirmDialog").modal('hide'); 
    

    This example might not be purely evil, but avoid accessing and modifying the DOM outside a Directive, you don't want to break other parts of your app by modifying the DOM underneath it.