Angular controller structure

Question

I'm a beginner with AngularsJs and I've got a question about the controller structure.

This is my employeeController.js

    (function()
    {
        angular.module('employeeApp').controller('employeeController', employeeController);

        function employeeController(employeeFactory,$routeParams,departmentFactory,schoolFactory,mentorFactory,constants,$location,$scope) {
            var vm = this;
            vm.employee = null;
            vm.employees = null;
            vm.profilePicture = null;
            vm.mentors = null;
            vm.schools = null;
            vm.departments = null;
            vm.roleId = constants.roleid;
            vm.internEducators = null;

            vm.overviewMentors = function() {
                mentorFactory.overview(constants.companyid).then(function(response)
                {
                    vm.mentors = response;
                });
            }

            vm.create = function()
            {
                employeeFactory.create(vm.employee,vm.profilePicture).success(function(response,status)
                {
                    if(status == 200)
                    {
                        $.toaster({message: 'De werknemer is toegevoegd'});
                        $location.path('/home');
                    }
                    }).error(function(response)
                    {
                        var i = 0;
                        vm.error = response;

                        angular.forEach(response.result.message, function(error)
                        {
                            if(i <= 2)
                            {
                                $.toaster({ priority: 'danger', message: error});
                            }
                            i++;
                        });
                    });
            }

            vm.overviewInternEducators = function() {
                employeeFactory.overviewInternEducators(constants.companyid).then(function(response)
                {
                    vm.internEducators = response;
                });
            }

            vm.overviewSchools = function() {
                schoolFactory.overview(constants.companyid).then(function(response)
                {
                    if(angular.isDefined(response[0].SchoolId))
                    {
                        vm.schools = response;
                    }
                    else
                    {
                        console.log('leeg!');
                    }
                });
            }

            vm.overviewDepartments = function() {
                console.log('test');
                departmentFactory.overview(constants.companyid).then(function(response)
                {
                    vm.departments = response;
                });
            }

            vm.show = function() {
                vm.isAdmin = employeeFactory.isAdmin();
                employeeFactory.show($routeParams.id).then(function(response)
                {
                    vm.employee = response;
                });
            }

            vm.showDeleted = function() {
                employeeFactory.showDeleted($routeParams.id).then(function(response)
                {
                    vm.employee = response;
                });
            }

            vm.update = function()
            {
                employeeFactory.update(vm.employee, vm.profilePicture).success(function(response, status)
                {
                    if(status == 200)
                    {
                        vm.show(); // Zodat de nieuwe afbeelding wordt weergegeven
                        $.toaster({ message: 'De werknemer is geupdated' });
                    }
                }).error(function(response)
                {
                    var i = 0;
                    vm.error = response;

                    angular.forEach(response.result.message, function(error)
                    {
                        if(i <= 2)
                        {
                            $.toaster({ priority: 'danger', message: error});
                        }
                        i++;
                    });
                });
            }

            vm.overviewDeleted = function() {
                employeeFactory.overviewDeleted().then(function(response)
                {
                    if(angular.isDefined(response[0].EmployeeId))
                    {
                        vm.employees = response;
                    }
                });
            }

            vm.delete = function() {
                employeeFactory.delete($routeParams.id).then(function(response)
                {
                    if(response == 'true')
                    {
                        $.toaster({ message: 'De werknemer is verwijderd' });

                        $location.path('/home');
                    }
                    else
                    {
                        angular.forEach(response, function(error)
                        {
                            $.toaster({ priority: 'danger', message: error });
                        });
                    }
                });

            }

            vm.permanentDelete = function(employeeId) {
                employeeFactory.permanentDelete(employeeId).then(function(response)
                {
                    if(response == 'true')
                    {
                        $.toaster({ message: 'De werknemer is permanent verwijderd' });

                        $location.path('/prullenbak/werknemers');
                    }
                    else
                    {
                        angular.forEach(response, function(error)
                        {
                            $.toaster({ priority: 'danger', message: error });
                        });
                    }
                });
            }
        vm.restore = function(employeeId) {
            employeeFactory.restore(employeeId).then(function(response)
            {
                if(response == 'true')
                {
                    $.toaster({ message: 'De werknemer is teruggezet' });
                    $location.path('/werknemer/' + employeeId);
                }
                else
                {
                    if(angular.isArray(response))
                    {
                        angular.forEach(response, function(error)
                        {
                            $.toaster({ priority : 'danger', message : error[0]});
                        });
                    }
                }
            });
        }

        <!--ng-init-->
        vm.editEmployee = function()
        {
            vm.show();
            vm.overviewDepartments();
            vm.overviewInternEducators();
            vm.overviewMentors();
            vm.overviewSchools();
        }

        vm.createEmployee = function() {
            vm.overviewMentors();
            vm.overviewSchools();
            vm.overviewDepartments();
            vm.overviewInternEducators();
        }

        <!--redirects-->
        vm.editRedirect = function(werknemerId)
        {
            $location.path('/werknemer/edit/' + werknemerId);
        }

        vm.showTrashedEmployeeRedirect = function(werknemerId)
        {
            $location.path('/prullenbak/werknemer/' + werknemerId);
        }
    }
})()

As you can see I use 2 methods called editEmployee and createEmployee (at the end). I use these 2 with the create employee page and the edit employee page. On both pages there are a couple of comboboxes that have to be loaded. In for example my create employee page I say ng-init="employeeController.createEmployee()" and then those comboboxes are filled.

I know this is not the best approach so how could I do this on a different and better way?

Thanks @jamie What exactly do you want help on? Is it the loading of the combo boxes or the whole structure of the app or controller ? — Evans Dianga
– Evans Dianga, Commented May 18, 2016 at 11:33
Thanks for your reply. Basically the structure of the controller. — Jamie
– Jamie, Commented May 18, 2016 at 11:35
Hi. I suggest you to check John Papa's Angular 1 style guide and, in particular, the controller section. — Andrea
– Andrea, Commented May 18, 2016 at 12:24

Evans Dianga · Accepted Answer · 2016-05-18 14:12:33Z

1

There are several ways to structure you app but the style guide endorsed by the Angular team is maintained by John Papa. Refer to Angular Style Guide by John Papa. Based on that:

First I would suggest that you split the create, show, edit and delete functionality into separate controllers. This helps with ensuring each controller does only one single thing.This is in line with the idea of Single Responsibility and Separation of Concerns

Secondly since it seems you are using the controllerAs syntax you wouldn't need to inject scope into your controller.

Here is the code for creating an employee(sth like create-employee.controller.js

(function () {
  'use strict';
   angular.module('employeeApp').controller('CreateEmployeeController', CreateEmployeeController);
  //ngInject is used to help create minify safe version of the file during the build process.
 //You should have this somewhere in your build process
/** @ngInject **/
function CreateEmployeeController(constants, departmentFactory, employeeFactory, $location, mentorFactory, schoolFactory) {
var vm = this;
vm.create = create;
getMentors();
getSchools();
getDepartments();
getInternEducators();

function getMentors() {
  return mentorFactory.overview(constants.companyid).then(function (response) {
    vm.mentors = response;
  });
}

function getSchools() {
  return schoolFactory.overview(constants.companyid).then(function (response) {
    if (angular.isDefined(response[0].SchoolId)) {
      vm.schools = response;
    }
    else {
      console.log('leeg!');
    }
  });
}

function getDepartments() {
  return departmentFactory.overview(constants.companyid).then(function (response) {
    vm.departments = response;
  });
}

function getInternEducators() {
  return employeeFactory.overviewInternEducators(constants.companyid).then(function (response) {
    vm.internEducators = response;
  });
}
}

function create() {
return employeeFactory.create(vm.employee, vm.profilePicture).success(function (response, status) {
  if (status == 200) {
    $.toaster({message: 'De werknemer is toegevoegd'});
    $location.path('/home');
  }
}).error(function (response) {
  var i = 0;
  vm.error = response;

  angular.forEach(response.result.message, function (error) {
    if (i <= 2) {
      $.toaster({priority: 'danger', message: error});
    }
    i++;
  });
});
}
})();

You would then create the other controllers by splitting the functionality following this example.

answered May 18, 2016 at 14:12

Evans Dianga

2102 silver badges11 bronze badges

Sign up to request clarification or add additional context in comments.

2 Comments

Jamie Over a year ago

Thankyou! Very useful. One more question the methods getMentors,getSchools,getDepartments are also used in edit employee. Should I just duplicate those methods?

Evans Dianga Over a year ago

@Jamie, That's something that I honestly do not know have a good answer to. I am leaning towards having them in a factory i.e.angular.module('myModule').factory('') and getting them in the controller i.e by injecting them in the controller. Another option is to repeat them in each controller but that goes against the DRY principle(Don't repeat yourself). Will dig into this and give an answer

Collectives™ on Stack Overflow

Angular controller structure

1 Answer 1

2 Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

1 Answer 1

2 Comments

Your Answer

Sign up or log in

Post as a guest

Related