Is it good practice to use directives for splitting controller logic in angular?

Question

I have pretty complicated controller (about 3K rows of code) that demonstrates dashboard. Controller contains a lot of charts, grid tables, and so on.

For example I moved grid table logic to undermentioned directive named wmGridActionItems. Notice, it uses parent scope:

app.directive('wmGridActionItems', ['$rootScope', '$timeout', function ($rootScope, $timeout) {
        return {
            restrict: 'E',
            templateUrl: 'views/grid-action-items.html',

            link: function (scope, elem, attrs) {

                // logic goes here
         }
        };
    }]);

and HTML:

<div  ui-grid="gridActionItemsOptions"                           
                      ui-grid-auto-resize
                      ui-grid-pagination
                      ui-grid-selection
                      ui-grid-auto-resize
                      ui-grid-resize-columns>
                </div>

So in main controller HTML i just write: <wm-grid-action-items></wm-grid-action-items>

I don't manage to use this directive in other places but at least I divide my BIG controller to several little directives that should help me to handle dashboard.

I do something wrong? Is it good practice? Has Angular other approaches to solve this issue?

EDIT

This is my $StateProvider for dashboard view:

$stateProvider  
 .state('sidemenu.dash', {
                    url: '/dshmngr',
                    abstract: true,
                    views: {
                        'content': {
                            templateUrl: 'views/dashboard/dashboard_manager.html',
                            controller: 'DashboardMngrCtrl'
                        }
                    }


                })

                .state('sidemenu.dash.main', {
                    url: '/main',
                    views: {
                        'content': {
                            templateUrl: 'views/dashboard/dashboard-main.html',
                            controller: 'DashboardNewCtrl'
                        }
                    }
                })


                .state('sidemenu.dash.drill', {
                    url: '/drill/:type',
                    views: {
                        'content': {
                            templateUrl: 'views/dashboard/dashboard-tag-details.html',
                            controller: 'DashboardDetailedCtrl'
                        }
                    }
                })

Thanks,

Answer 1

You are aiming at the right direction. Breaking a large controller into more smaller components in the form of directives is the way to go but I would suggest you to introduce few changes.

1. Isolate the scope of directive and define the data that directive expects explicitly. That way one can instantly see what data directive accepts.

2. For easier testing pair the directive with Controller.

Based on the above two suggestions, your directive should look something like this:

app.directive('wmGridActionItems', [function () {
        return {
            controller: 'WmGridActionItemsController'
            restrict: 'E',
            templateUrl: 'views/grid-action-items.html',
            scope: {
               gridActionItemsOptions: '=gridActionItemsOptions' 
            }
            link: function (scope, elem, attrs) {
                // DOM manpulation (if needed) goes here
            }
        };
    }]);

app.controller('WmGridActionItemsController', ['$cope', '$timeout', function ($cope, $timeout) {
    // logic goes here
}]);

You would then call above directive like:

<wm-grid-action-items grid-action-item-options="gridActionItemsOptions">
</wm-grid-action-items>

I suggest you to also read this great blog post which explains "Component Pattern" in details.

Also note that sharing a model by explicitly specifying it when defining an isolated scope is not the only way. Another alternative for sharing a data would for example be a Model as a service (please see related reading).

Answer 2

The "good practice" that I would recommend to you is the single responsibility principle.

Every components (directive/controller/service) that you build MUST never do more than one thing. If you avoid this error, your components will be way more reusable, readable and maintanable.

Of course, that practice is not only for angular.

One you respect that, I would recommend you to :

• Avoid to put all your business code in controllers, and to use instead services (or providers). Services are way more powerfull as they allow to use the angular dependency injection system.

• Directives should contain only DOM manipulation.

Angular Directive/Controller/Service is kind of a View/ViewModel/Model pattern. Try to keep that in mind.

Edit: Every of your directives can have a controller. You can put directives inside an other directive template and then use the last parameter of the link function (controllers) and the directive require parameter for the communication between directives.

Example: (coffeescript) Let's say i have a container that can be inside others containers itself and may contain also a widget :

angular.module('dashboard')
.directive('dashboardWidget', [() ->
    restrict: 'E'
    templateUrl : '/views/dashboard/widget.html'
    require: ['^^dashboardContainer']
    scope:
        'model': '='
    controller: 'DashboardWidgetController'
    controllerAs: 'DashboardWidget'
    # default => post link (children already instanciated)
    link: ($scope, element, attrs, ctrls) ->
        [parentDashboardContainerController] = ctrls

        # some very small code (mainly events), the real code is in the controller

        return
])

angular.module('dashboard')
.directive('dashboardContainer', [() ->
    restrict: 'E'
    templateUrl : '/views/dashboard/container.html'
    require: ['?^^dashboardContainer', '?^ngController']
    scope:
        'model': '='
    controller: 'DashboardContainerController'
    controllerAs: 'DashboardContainer'
    # default => post link (children already instanciated)
    link: ($scope, element, attrs, ctrls) ->
        [parentDashboardContainerController, ngController] = ctrls

        # some very small code (mainly events), the real code is in the controller

        return
])