optimise angular duplicate code

Question

How can i optimise the duplicate code here

angular.module('myApp')
    .controller('LogsController', function ($scope, LogsService) {
        $scope.updatingLogs = true;
        $scope.loggers = {};

        LogsService.findAll().$promise.then(function(data) {
            $scope.loggers = data;
            $scope.updatingLogs = false;
        });

        $scope.changeLevel = function (name, level) {
            LogsService.changeLevel({name: name, level: level}, function () {
                $scope.updatingLogs = true;
                LogsService.findAll().$promise.then(function(data) {
                    $scope.loggers = data;
                    $scope.updatingLogs = false;
                });
            });
        };
    });

Daniel Silva · Accepted Answer · 2015-04-30 22:20:07Z

1

Here is a suggestion:

angular.module('myApp')
    .controller('LogsController', function ($scope, LogsService) {
        $scope.updatingLogs;
        $scope.loggers = LogsService.findAll();

        $scope.changeLevel = function (name, level) {
            $scope.updatingLogs = LogsService.changeLevel({name: name, level: level}, function () {
                LogsService.findAll().$promise.then(function(data) {
                    $scope.loggers = data;
                    $scope.updatingLogs = null;
                });
            });
        };
    });

answered Apr 30, 2015 at 22:20

Daniel Silva

3791 silver badge11 bronze badges

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

Comments

Pankaj Parkar · Accepted Answer · 2015-05-06 14:29:34Z

0

You could do create $scope.findAll method which will make an ajax of LogsService.findAll() so that you could be utilize this function while doing it multiple times from elsewhere.Optimize version would be.

Code

angular.module('myApp')
    .controller('LogsController', function($scope, LogsService) {
        $scope.updatingLogs = true;
        $scope.loggers = {};

        $scope.findAll = function() {
            LogsService.findAll().$promise.then(function(data) {
                $scope.loggers = data;
                $scope.updatingLogs = false;
            });
        }

        $scope.changeLevel = function(name, level) {
            LogsService.changeLevel({
                name: name,
                level: level
            }, function() {
                $scope.updatingLogs = true;
                $scope.findAll();
            });
        };

        $scope.findAll(); //init
    });

edited May 6, 2015 at 14:29

answered Apr 30, 2015 at 22:03

Pankaj Parkar

136k23 gold badges241 silver badges307 bronze badges

2 Comments

coblr Over a year ago

this is really close to what I was about to say. The difference was that findAll() wasn't in $scope, unless the view needed to call it directly.

Pankaj Parkar Over a year ago

@fractalspawn ya..i added it in $scope because either way we can initialize it using ng-init which would make more sense as per unit testing perspective

domitall · Accepted Answer · 2015-04-30 22:34:18Z

Depends on what you're reaching for. If you're looking for code clarity, a potential refactor might look something like this.

angular.module('myApp')
  .controller('LogsController', function ($scope, LogsService) {

    activate();

    $scope.changeLevel = function (name, level) {
        LogsService.changeLevel({name: name, level: level}, changelevelHandler);
    }; 

    function changeLevelHandler() {
        $scope.updatingLogs = true;
        getLogs();
    }

    function getLogs() {
      LogsService.findAll().$promise.then(function(data) {updateLoggers(data);});
    }

    function updateLoggers(data) {
        $scope.loggers = data;
        $scope.updatingLogs = false;
    }

    function activate() {
       $scope.updatingLogs = true;
       $scope.loggers = {};
       getLogs();

    }
});

Collectives™ on Stack Overflow

optimise angular duplicate code

3 Answers 3

Comments

2 Comments

Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

3 Answers 3

Comments

2 Comments

Comments

Your Answer

Sign up or log in

Post as a guest

Related