6

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,

snaggs
  • 5,543
  • 17
  • 64
  • 127
  • Rather than putting your main controller's logic in a directive, why don't you make _another_ controller and put the logic there? Nothing's preventing you from having multiple controllers. – jperezov Feb 19 '16 at 17:42
  • well, I try to solve current issue. Generally I have 3 controllers with abstract routing and several sub-views. Each view has own controller. but splitting one view to several controllers will cause logic too complicated, broadcasts, watchers, and so on – snaggs Feb 19 '16 at 17:44
  • as a good practice you won't want to do much of the logic in controller. And this is not applying only for angular but for the hole MVC frameworks. So you will want to break the logic in directive in factories or in services. As @jperov said you can have multiple controller and even inherit from one to other – Radu Feb 19 '16 at 17:44
  • @snaggs just seeing your comment your can consider to inherit the controllers in your sub-views. you can read more here:http://stackoverflow.com/questions/15386137/angularjs-controller-inheritance – Radu Feb 19 '16 at 17:48

2 Answers2

3

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).

PrimosK
  • 13,848
  • 10
  • 60
  • 78
1

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
])
antoinestv
  • 3,286
  • 2
  • 23
  • 39
  • when I say `about 3K rows of code` means controller logic NETO. a.e. UI. all business logic lays under services and factories, indeed – snaggs Feb 19 '16 at 17:46
  • I know well MVC model and I know that directive is UI responsibility and DOM manipulations .... – snaggs Feb 19 '16 at 17:47
  • Each of your directives can have a controller (with the 'controller' property). So split your code with the single responsability principe and that should be much cleaner. Personally if one of my file contains more than 200 lines, I start wondering myself if it doesn't do too much things. That is something very important, especially in big projects. You will one day come back to your files so make them easy to read. – antoinestv Feb 19 '16 at 17:50
  • agree, like one function size should enter to one screen :) – snaggs Feb 19 '16 at 17:52
  • I added some simple examples. – antoinestv Feb 19 '16 at 17:59