How to organize and reduce AngularJS controller size?

Question

I am building my first AngularJS app and am struggling to figure out how to organize my code into services/factories and directives. My controller code keeps getting longer and longer and I know I must be doing something wrong.

Originally my app had three controllers: "BuildingTypeController", "AddressController", and "LayoutController". I used the ng-controller directive to attach each controller to each of my fieldsets. However I encountered problems when:

I needed to pass one controller's data to another controller to be able to use in the $scope.$watch code.
When I needed to pass one controller's data to another controller in my Get... $http.get methods (replaced with hardcoded values in my code below).

So then I went back and took all of my properties and functions and through them in one big "FormController". This gives me the functionality I'm looking for, however I know this can't possibly be the right way to organize all of this code.

If anyone could offer any advice on how to structure a long controller into smaller pieces I would greatly appreciate it. Thank you!

EDIT: The hardcoded datasets in this code were created to allow for the fiddle to work - they will be replaced with database calls and should not be factored into the "length" of the single controller.

var app = angular.module('formBuilder', []);

app.controller('FormController', ['$http', '$scope', function ($http, $scope) {
    
    //This could really be thought of as "BuildingTypeController"
    var form = this;

    form.state = "";
    form.states = [];

    form.buildingType = "";
    form.buildingTypes = [];

    form.GetStates = function (buildingType) {
        //This will be replaced with $http.get later
        if (buildingType == null)
        {
         	form.states = ["AZ","CT","NY"];   
        }
        if (buildingType == "Single Story") {
            form.states = ["AZ","CT"];
        }
        if (buildingType == "Mansion") {
            form.states = ["CT"];
        }
        if (buildingType == "Apartment") {
            form.states = ["CT","NY"];
        }
        if (buildingType == "Sky Scraper") {
            form.states = ["NY"];
        }
        
    };

    form.GetBuildingTypes = function (state) {
        //This will be replaced with $http.get later
        if (state == null)
        {
         	form.buildingTypes = ["Single Story","Mansion","Apartment","Sky Scraper"];   
        }
        if (state == "AZ") {
            form.buildingTypes = ["Single Story"];
        }
        if (state == "CT") {
            form.buildingTypes = ["Single Story", "Mansion","Apartment"];
        }
        if (state == "NY") {
            form.buildingTypes = ["Sky Scraper", "Apartment"];
        }
    };

    //initializations
    form.GetStates();
    form.GetBuildingTypes();

    //This could really be thought of as "AddressController"
    form.addressId = "";
    form.addresses = [];

    form.GetAddresses = function (form) {
        //This will be replaced with $http.get later
        if (form.state == "AZ" && form.buildingType == "Single Story") {
            form.addresses = [{
                addressId: 1,
                description: "123 Grove Ave"
            },{
                addressId: 2,
                description: "2352 High Court"
            }];
        }
        if (form.state == "CT" && form.buildingType == "Single Story") {
            form.addresses = [{
                addressId: 3,
                description: "1515 Lark Ave"
            },{
                addressId: 4,
                description: "2 Front St"
            }];
        }
        if (form.state == "CT" && form.buildingType == "Mansion") {
            form.addresses = [{
                addressId: 5,
                description: "6 Waterfront Dr"
            }]
        }
        if (form.state == "CT" && form.buildingType == "Apartment") {
            form.addresses = [{
                addressId: 6,
                description: "13 Center St"
            },{
                addressId: 7,
                description: "5985 Elizabeth Court "
            }]
        }
        if (form.state == "NY" && form.buildingType == "Sky Scraper") {
            form.addresses = [{
                addressId: 8,
                description: "13245 12th Ave"
            },{
                addressId: 9,
                description: "345 Park Ave"
            }]
        }
        
        if (form.state == "NY" && form.buildingType == "Apartment") {
            form.addresses = [{
                addressId: 10,
                description: "6668 115th St"
            },{
                addressId: 11,
                description: "2839 3rd Ave"
            }]
        }

    };

    form.performAction = function (expr) {
        return function () {
            if (expr == "GetAddresses") {
                form.GetAddresses($scope.form);
            }
            if (expr == "GetLayouts") {
                form.GetLayouts($scope.form);
            }
        };
    };

    $scope.$watch('form.state', form.performAction('GetAddresses'));
    $scope.$watch('form.buildingType', form.performAction('GetAddresses'));

    
    //This could really be thought of as "LayoutController"
    form.layout = {};
    form.layouts = [];

    form.GetLayouts = function (form) {
        if (form.addressId == 1)
        	form.layouts = ["A", "B", "C", "D"];
        if (form.addressId == 2)
        	form.layouts = ["B", "C", "D"];
        if (form.addressId == 3)
        	form.layouts = ["A", "D"];
        if (form.addressId == 4)
        	form.layouts = ["A", "D"];
        if (form.addressId == 5)
        	form.layouts = ["A"];
        if (form.addressId == 6)
        	form.layouts = ["D"];
        if (form.addressId == 7)
        	form.layouts = ["C", "D"];
        if (form.addressId == 8)
        	form.layouts = ["A", "D"];
        if (form.addressId == 9)
        	form.layouts = ["A", "B"];
        if (form.addressId == 10)
        	form.layouts = ["B", "C", "D"];
        if (form.addressId == 11)
        	form.layouts = ["C", "D"];
    };

    $scope.$watch('form.addressId', form.performAction('GetLayouts'));

}]);

<script src="https://ajax.googleapis.com/ajax/libs/angularjs/1.2.23/angular.min.js"></script>
<div ng-app="formBuilder">
    <form name="createForm" class="form-horizontal" ng-controller="FormController as form" novalidate>
        <fieldset>
            <legend>Building Type</legend>
            <select class="form-control" ng-model="form.state" ng-options="state for state in form.states" ng-change="form.GetBuildingTypes(form.state)">
                <option value="">Select a state</option>
            </select>
            <select class="form-control" ng-model="form.buildingType" ng-options="buildingType for buildingType in form.buildingTypes" ng-change="form.GetStates(form.buildingType)">
                <option value="">Select a building type</option>
            </select>
        </fieldset>
        
        <fieldset>
            <legend>Specific Address</legend>
            <select class="form-control" ng-model="form.addressId" ng-options="obj.addressId as obj.description for obj in form.addresses">
                <option value="">Select an address</option>
            </select>
        </fieldset>
        
        <fieldset>
            <legend>Select Room</legend>
            <select class="form-control" ng-model="form.layout" ng-options="layout for layout in form.layouts">
                <option value="">Select a room</option>
            </select>
        </fieldset>
    </form>
</div>

Many of your datasets, like the hard-coded arrays could be abstracted out into their own JavaScript files. — idream1nC0de
– idream1nC0de, Commented Sep 1, 2015 at 19:03
Just as a side note, consider using a switch for the form.GetLayouts method. It's more efficient and easier to read than the multiple if statements. If you insist on using the if statements, why not use else if, instead of if subsequently so that each statement does not get analyzed when a condition is already met? — idream1nC0de
– idream1nC0de, Commented Sep 1, 2015 at 19:44
This is more a question for stackexchange CodeReview. Add your question there and I can write you an answer and describe my jsFiddle. Please have a look at this fiddle so you'll get an idead how you could refactor your code. — AWolf
– AWolf, Commented Sep 1, 2015 at 22:04
Please note, if this is migrated to Code Review, the title must describe the purpose of the code, not asking to improve it. — Ethan Bierlein
– Ethan Bierlein, Commented Sep 1, 2015 at 22:05

idream1nC0de · Accepted Answer · 2015-09-01 19:41:01Z

One way you could reduce the number of lines in the controller is to abstract out some of the hard-coded datasets into a static JavaScript file. Anything you feel that may be heavy in the controller that could be abstracted out could potentially be an area for improvement.

An example could be to create a global data variable where you could access the variables on that page.

(function(global){
  global.data = {
    names: ['Jane', 'John', 'Mary', 'Michael', 'Ryan', 'Rachel']
  };
})(this);

Here's an example of usage in the controller.

(function(global){
  app.controller('MainController', function($log){
    this.message = "Hello, world!";
    //Usage of the global variable here.
    this.names = global.data.names;
    this.click = function(){
      global.handlers.onclick(this);
    };
  });
})(this);

Here's an example of usage in the view.

<!DOCTYPE html>
<html ng-app="TestModule">

  <head>
    <link rel="stylesheet" href="style.css">
    <!-- Reference to Angular on CDN -->
    <script src="https://cdnjs.cloudflare.com/ajax/libs/angular.js/1.4.5/angular.min.js"></script>
    <!-- Reference to external data sets -->
    <script src="data.js"></script>
    <!-- Reference to external event handlers -->
    <script src="handlers.js"></script>
    <!-- Angular module init -->
    <script src="module.js"></script>
    <!-- Controller init -->
    <script src="maincontroller.js"></script>
  </head>

  <body>
    <div ng-controller="MainController as main">
      <h3>{{ main.message }}</h3>
      <div>
        <h2>Names from the global data.js file:</h2>
        <h3 ng-repeat="n in main.names">{{ n }}</h3>
      </div>
      <div>
        <input type="button" ng-click="main.click()" value="Click me!" />
      </div>
    </div>
  </body>

</html>

See my plunker for a working example:

http://plnkr.co/edit/seCQiNpYwKbgjkngdCFz?p=preview

U r s u s · Accepted Answer · 2015-09-01 19:49:16Z

0

Here's three basic things to do:

abstract your data in new files
use services to make your controllers agnostic as to how data is retrieved
(less basic) use UI Router to split your app into states

This is an excellent resource for going into more details.

answered Sep 1, 2015 at 19:49

U r s u s

7,05812 gold badges54 silver badges88 bronze badges

Comments

Aleksey Pastuhov · Accepted Answer · 2015-09-01 21:00:42Z

0

You can use any decorators, libraries or other languages. In example I wrote my own library to optimize work with AngularJS and TypeScript.

https://github.com/aleksey-pastuhov/AngularJS-Typed-Core

P.S.: Examples do not works now, but I will fix it soon.

answered Sep 1, 2015 at 21:00

Aleksey Pastuhov

1661 silver badge7 bronze badges

Collectives™ on Stack Overflow

How to organize and reduce AngularJS controller size?

3 Answers 3

Comments

Comments

Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

3 Answers 3

Comments

Comments

Comments

Your Answer

Sign up or log in

Post as a guest

Related