AngularJS controllers, design pattern for a DRY code

Question

I have created a full example for the purpose of describing this issue. My actual application is even bigger than the presented demo and there are more services and directives operated by every controller. This leads to even more code repetition. I tried to put some code comments for clarifications, PLUNKER: http://plnkr.co/edit/781Phn?p=preview

Repetitive part:

routerApp.controller('page1Ctrl', function(pageFactory) {
  var vm = this;

  // page dependent
  vm.name = 'theOne';
  vm.service = 'oneService';
  vm.seriesLabels = ['One1', 'Two1', 'Three1'];

  // these variables are declared in all pages
  // directive variables,
  vm.date = {
    date: new Date(),
    dateOptions: {
      formatYear: 'yy',
      startingDay: 1
    },
    format: 'dd-MMMM-yyyy',
    opened: false
  };

  vm.open = function($event) {
    vm.date.opened = true;
  };

  // dataservice
  vm.data = []; // the structure can be different but still similar enough
  vm.update = function() {
      vm.data = pageFactory.get(vm.service);
    }

  //default call
  vm.update();   
})

---

Basically I moved all the logic I could to factories and directives. But now in every controller that uses certain directive I need, for example, a field that keeps the value that directive is modifying. And it's settings. Later I need similar field to keep the data that comes from dataservice, and the call itself (method) is the same as well.

This leads to a lot of repetition.

---

Graphically I see the current example to look like this:

While I believe the proper design should look more like this:

---

I tried to find some solution here, but none seem to be confirmed. What I have found:

1. AngularJS DRY controller structure, suggesting I pass the $scope or vm and decorate it with extra methods and fields. But many sources say it is dirty solution.
2. What's the recommended way to extend AngularJS controllers? using angular.extend, but this have problems when using controller as syntax.
3. And then I have found also the answer (in the link above):

You don't extend controllers. If they perform the same basic functions then those functions need to be moved to a service. That service can be injected into your controllers.

And even when I did there is still a lot of repetition. Or is it the way it just has to be? Like John Papa sais (http://www.johnpapa.net/angular-app-structuring-guidelines/):

Try to stay DRY (Don't Repeat Yourself) or T-DRY

Did you face a similar issue? What are the options?

Answer 1

From a over all design perspective I don't see much of a difference between decorating a controller and extending a controller. In the end these are both a form of mixins and not inheritance. So it really comes down to what you are most comfortable working with. One of the big design decisions comes down to not just how to pass in functionality to just all of the controllers, but how to also pass in functionality to say 2 out of the 3 controllers also.

Factory Decorator

One way to do this, as you mention, is to pass your $scope or vm into a factory, that decorates your controller with extra methods and fields. I don't see this as a dirty solution, but I can understand why some people would want to separate factories from their $scope in order to separate concerns of their code. If you need to add in additional functionality to the 2 out of 3 scenario, you can pass in additional factories. I made a plunker example of this.

dataservice.js

routerApp.factory('pageFactory', function() {

    return {
      setup: setup
    }

    function setup(vm, name, service, seriesLabels) {
      // page dependent
      vm.name = name;
      vm.service = service;
      vm.seriesLabels = seriesLabels;

      // these variables are declared in all pages
      // directive variables,
      vm.date = {
        date: moment().startOf('month').valueOf(),
        dateOptions: {
          formatYear: 'yy',
          startingDay: 1
        },
        format: 'dd-MMMM-yyyy',
        opened: false
      };

      vm.open = function($event) {
        vm.date.opened = true;
      };

      // dataservice
      vm.data = []; // the structure can be different but still similar enough
      vm.update = function() {
        vm.data = get(vm.service);
      }

      //default call
      vm.update();
    }

});

page1.js

routerApp.controller('page1Ctrl', function(pageFactory) {
    var vm = this;
    pageFactory.setup(vm, 'theOne', 'oneService', ['One1', 'Two1', 'Three1']);
})

Extending controller

Another solution you mention is extending a controller. This is doable by creating a super controller that you mix in to the controller in use. If you need to add additional functionality to a specific controller, you can just mix in other super controllers with specific functionality. Here is a plunker example.

ParentPage

routerApp.controller('parentPageCtrl', function(vm, pageFactory) {

    setup()

    function setup() {

      // these variables are declared in all pages
      // directive variables,
      vm.date = {
        date: moment().startOf('month').valueOf(),
        dateOptions: {
          formatYear: 'yy',
          startingDay: 1
        },
        format: 'dd-MMMM-yyyy',
        opened: false
      };

      vm.open = function($event) {
        vm.date.opened = true;
      };

      // dataservice
      vm.data = []; // the structure can be different but still similar enough
      vm.update = function() {
        vm.data = pageFactory.get(vm.service);
      }

      //default call
      vm.update();
    }

})

page1.js

routerApp.controller('page1Ctrl', function($controller) {
    var vm = this;
    // page dependent
    vm.name = 'theOne';
    vm.service = 'oneService';
    vm.seriesLabels = ['One1', 'Two1', 'Three1'];
    angular.extend(this, $controller('parentPageCtrl', {vm: vm}));
})

Nested States UI-Router

Since you are using ui-router, you can also achieve similar results by nesting states. One caveat to this is that the $scope is not passed from parent to child controller. So instead you have to add the duplicate code in the $rootScope. I use this when there are functions I want to pass through out the whole program, such as a function to test if we are on a mobile phone, that is not dependent on any controllers. Here is a plunker example.

Answer 2

You can reduce a lot of your boilerplate by using a directive. I've created a simple one to replace all of your controllers. You just pass in the page-specific data through properties, and they will get bound to your scope.

routerApp.directive('pageDir', function() {
  return {
    restrict: 'E',
    scope: {},
    controller: function(pageFactory) {
      vm = this;
      vm.date = {
        date: moment().startOf('month').valueOf(),
        dateOptions: {
          formatYear: 'yy',
          startingDay: 1
        },
        format: 'dd-MMMM-yyyy',
        opened: false
      };

      vm.open = function($event) {
        vm.date.opened = true;
      };

      // dataservice
      vm.data = []; // the structure can be different but still similar enough
      vm.update = function() {
        vm.data = pageFactory.get(vm.service);
      };

      vm.update();
    },
    controllerAs: 'vm',
    bindToController: {
      name: '@',
      service: '@',
      seriesLabels: '='
    },
    templateUrl: 'page.html',
    replace: true
  }
});

As you can see it's not much different than your controllers. The difference is that to use them, you'll use the directive in your route's template property to initialize it. Like so:

    .state('state1', {
        url: '/state1',
        template: '<page-dir ' +
          'name="theOne" ' +
          'service="oneService" ' +
          'series-labels="[\'One1\', \'Two1\', \'Three1\']"' +
          '></page-dir>'
    })

And that's pretty much it. I forked your Plunk to demonstrate. http://plnkr.co/edit/NEqXeD?p=preview

EDIT: Forgot to add that you can also style the directive as you wish. Forgot to add that to the Plunk when I was removing redundant code.

Answer 3

I can't respond in comment but here what i will do :

I will have A ConfigFactory holding a map of page dependent variables :

{
  theOne:{
      name: 'theOne',
      service: 'oneService',
      seriesLabels: ['One1', 'Two1', 'Three1']
  },
  ...
}

Then i will have a LogicFactory with a newInstance() method to get a proper object each time i need it. The logicFactory will get all the data / method shared betwwen controllers. To this LogicFactory, i will give the view-specific data. and the view will have to bind to this Factory.

And to retrieve the view-specific data i will pass the key of my configuration map in the router.

so let say the router give you #current=theOne, i will do in the controller :

var specificData = ServiceConfig.get($location.search().current);
this.logic = LogicFactory.newInstance(specificData);

Hope it help

I retouch your example, here is the result : http://plnkr.co/edit/ORzbSka8YXZUV6JNtexk?p=preview

Edit: Just to say this way, you can load the specific configuration from a remote server serving you the specific-view data

Answer 4

I faced completely the same issues as you described. I'm a very big supporter of keeping things DRY. When I started using Angular there was no prescribed or recommended way to do this, so I just refactored my code as I went along. As with many things I dont think their is a right or wrong way to do these things, so use whichever method you feel comfortable with. So below is what I ended up using and it has served me well.

In my applications I generally have three types of pages:

1. List Page - Table list of specific resource. You can search/filter/sort your data.
2. Form Page - Create or Edit resource.
3. Display Page - Detailed view-only display page of resource/data.

I've found there are typically a lot of repetitive code in (1) and (2), and I'm not referring to features that should be extracted to a service. So to address that I'm using the following inheritance hierarchy:

1. List Pages

   • BaseListController
     • loadNotification()
     • search()
     • advancedSearch()
     • etc....
   • ResourceListController
     • any resource specific stuff

2. Form Pages

   • BaseFormController
     • setServerErrors()
     • clearServerErrors()
     • stuff like warn user is navigating away from this page before saving the form, and any other general features.
   • AbstractFormController
     • save()
     • processUpdateSuccess()
     • processCreateSuccess()
     • processServerErrors()
     • set any other shared options
   • ResourceFormController
     • any resource specific stuff

To enable this you need some conventions in place. I typically only have a single view template per resource for Form Pages. Using the router resolve functionality I pass in a variable to indicate if the form is being used for either Create or Edit purposes, and I publish this onto my vm. This can then be used inside your AbstractFormController to either call save or update on your data service.

To implement the controller inheritance I use Angulars $injector.invoke function passing in this as the instance. Since $injector.invoke is part of Angulars DI infrastructure, it works great as it will handle any dependencies that the base controller classes need, and I can supply any specific instance variables as I like.

Here is a small snippet of how it all is implemented:

Common.BaseFormController = function (dependencies....) {
    var self = this;
    this.setServerErrors = function () {
    };
    /* .... */
};

Common.BaseFormController['$inject'] = [dependencies....];

Common.AbstractFormController = function ($injector, other dependencies....) {
    $scope.vm = {};
    var vm = $scope.vm;
    $injector.invoke(Common.BaseFormController, this, { $scope: $scope, $log: $log, $window: $window, alertService: alertService, any other variables.... });
   /* ...... */
}

Common.AbstractFormController['$inject'] = ['$injector', other dependencies....];

CustomerFormController = function ($injector, other dependencies....) {
    $injector.invoke(Common.AbstractFormController, this, {
            $scope: $scope,
            $log: $log,
            $window: $window,
            /* other services and local variable to be injected .... */
        });

    var vm = $scope.vm;
    /* resource specific controller stuff */
}

CustomerFormController['$inject'] = ['$injector', other dependencies....];

To take things a step further, I found massive reductions in repetitive code through my data access service implementation. For the data layer convention is king. I've found that if you keep a common convention on your server API you can go a very long way with a base factory/repository/class or whatever you want to call it. The way I achieve this in AngularJs is to use a AngularJs factory that returns a base repository class, i.e. the factory returns a javascript class function with prototype definitions and not an object instance, I call it abstractRepository. Then for each resource I create a concrete repository for that specific resource that prototypically inherits from abstractRepository, so I inherit all the shared/base features from abstractRepository and define any resource specific features to the concrete repository.

I think an example will be clearer. Lets assume your server API uses the following URL convention (I'm not a REST purest, so we'll leave the convention up to whatever you want to implement):

GET  -> /{resource}?listQueryString     // Return resource list
GET  -> /{resource}/{id}                // Return single resource
GET  -> /{resource}/{id}/{resource}view // Return display representation of resource
PUT  -> /{resource}/{id}                // Update existing resource
POST -> /{resource}/                    // Create new resource
etc.

I personally use Restangular so the following example is based on it, but you should be able to easily adapt this to $http or $resource or whatever library you are using.

AbstractRepository

app.factory('abstractRepository', [function () {

    function abstractRepository(restangular, route) {
        this.restangular = restangular;
        this.route = route;
    }

    abstractRepository.prototype = {
        getList: function (params) {
            return this.restangular.all(this.route).getList(params);
        },
        get: function (id) {
            return this.restangular.one(this.route, id).get();
        },
        getView: function (id) {
            return this.restangular.one(this.route, id).one(this.route + 'view').get();
        },
        update: function (updatedResource) {
            return updatedResource.put();
        },
        create: function (newResource) {
            return this.restangular.all(this.route).post(newResource);
        }
        // etc.
    };

    abstractRepository.extend = function (repository) {
        repository.prototype = Object.create(abstractRepository.prototype);
        repository.prototype.constructor = repository;
    };

    return abstractRepository;
}]);

Concrete repository, let's use customer as an example:

app.factory('customerRepository', ['Restangular', 'abstractRepository', function (restangular, abstractRepository) {

    function customerRepository() {
        abstractRepository.call(this, restangular, 'customers');
    }

    abstractRepository.extend(customerRepository);
    return new customerRepository();
}]);

So now we have common methods for data services, which can easily be consumed in the Form and List controller base classes.

Answer 5

To summarize the previous answers:

1. Decorating controllers: as you said, this is a dirty solution; Imagine having different factories decorating the same controller, it will be very difficult (especially for other developers) to prevent collision of properties, and equally difficult to trace which factory added which properties. It's actually like having multiple inheritance in OOP, something that most modern languages prevent by design for the same reasons.

2. Using a directive: this can be a great solution if all your controllers are going to have the same html views, but other than that you will have to include fairly complex logic in your views which can be difficult to debug.

---

The approach I propose is using composition (instead of inheritance with decorators). Separate all the repetitive logic in factories, and leave only the creation of the factories in the controller.

routerApp.controller('page1Ctrl', function (Page, DateConfig, DataService) {
    var vm = this;

    // page dependent
    vm.page = new Page('theOne', 'oneService', ['One1', 'Two1', 'Three1']);

    // these variables are declared in all pages
    // directive variables,
    vm.date = new DateConfig()

    // dataservice
    vm.dataService = new DataService(vm.page.service);

    //default call
    vm.dataService.update();

})

.factory('Page', function () {

    //constructor function
    var Page = function (name, service, seriesLabels) {
        this.name = name;
        this.service = service;
        this.seriesLabels = seriesLabels;
    };

    return Page;

})


.factory('DateConfig', function () {

    //constructor function
    var DateConfig = function () {
        this.date = new Date();
        this.dateOptions = {
            formatYear: 'yy',
            startingDay: 1
        };
        this.format = 'dd-MMMM-yyyy';
        this.opened = false;
        this.open = function ($event) {
            this.opened = true;
        };
    };

    return DateConfig;

})

This code is not tested, but I just want to give an idea. The key here is to separate the code in the factories, and add them as properties in the controller. This way the implementation is not repeated (DRY), and everything is obvious in the controller code.

You can make your controller even smaller by wrapping all the factories in a larger factory (facade), but this may make them more tightly coupled.