patternjavascriptMinor

Beginner's Calculator code

Submitted by: @import:stackexchange-codereview·Mar 10, 2026·

Viewed 0 times

codebeginnercalculator

Problem

I am trying to learn AngularJS as a start to my revival of learning, I think that I am getting the hang of it so far, but would like to see what I can do differently with some super simple applications, hopefully I can see what I need to work on this way.

First thing is first, a basic calculator.

This is a super basic calculator, I haven't done anything super difficult yet. I did create 2 tables that each do something different, and only show when appropriate, one is a multiplication table and the other is a squares table.

other than that everything does what it should do so far.

`var calculatorApp = angular.module("calculatorApp", []);

calculatorApp.factory('MathService', function() {
var factory = {};

factory.multiply = function(a, b) {
return a * b;
}
factory.add = function(a, b) {
return a + b;
}
factory.subtract = function(a, b) {
return a - b;
}
factory.divide = function(a, b) {
return a / b;
}

return factory;
});

calculatorApp.service('calcService', function(MathService) {
this.square = function(a) {
return MathService.multiply(a, a);
}
this.add = function(a, b) {
return MathService.add(a, b);
}
this.subtract = function(a, b) {
return MathService.subtract(a, b);
}
this.divide = function(a, b) {
return MathService.divide(a, b);
}
this.multiply = function(a, b) {
return MathService.multiply(a, b);
}
});

calculatorApp.controller('calcController', function($scope, calcService) {
$scope.squared = function() {
$scope.result = $scope.square($scope.number);
$scope.showSquares = true;
$scope.showMultiplicationTable = false;
}
$scope.add = function() {
$scope.output = calcService.add($scope.operand1, $scope.operand2);
$scope.showMultiplicationTable = false;
$scope.showSquares = false;
}
$scope.subtract = function() {
this.output = calcService.subtract(this.operand1, this.operand2);
$scope.showMultiplicationTable = false;
$scope.showS

Solution

Alright - A couple of things:

Your calcService is redundant. The mathService is just fine.

Try to avoid $scope and just use controller as. This link explains it very well.

You have two variables which is either true of false, depending on what function is called. make a small utility function that you call instead.

Right now you have everything in one file. Split it up into several (in your case: 3). One main file where you declare the module. One for the service and one for the controller.

At the bottom of your controller, you have some initialization, where you fill and array. Move that into an init() function you call instead.

So, with that in mind. Here is a revised version of your code, following best practices from this guide (highly recommended):

core.js

(function(){
    angular.module('calculatorApp', []);
})();

calcService.js

(function(){

    angular.module('calculatorApp')
    .factory('calcService', calcService);

    function calcService() {
        var formulas = {
            square: square,
            multiply: multiply,
            add: add,
            subtract: subtract,
            divide: divide
        };

        return formulas;

        function square(a) {
            return formulas.multiply(a, a);
        }

        function multiply(a, b) {
            return a * b;
        }

        function add(a, b) {
            return a + b;
        }

        function subtract(a, b) {
            return a - b;
        }

        function divide(a, b) {
            return a / b;
        }

    }

})();

calcController.js

(function() {

    angular.module('calculatorApp')
    .controller('calcController', calcController);

    calcController.$inject = ['calcService'];

    function calcController(calcService) {
        var vm = this;

        vm.showMultiplicationTable = false;
        vm.showSquares = false;
        vm.output = null;
        vm.result = null;
        vm.k = [];
        vm.operand1 = null;
        vm.operand2 = null;

        vm.squared = squared;
        vm.square = square;
        vm.add = add;
        vm.subtract = subtract;
        vm.divide = divide;
        vm.multiply = multiply;

        init();

        function init() {
            console.log(vm.result);
            var arrayLength = 25,
                k = [];

            for (var i = 0; i < arrayLength; i++) k[i] = i;

            vm.k = k;   
        }

        function squared(number) {
            vm.result = calcService.square(number);
            setPropertiesVisible(false, true);
        }

        function square(i) {
            return calcService.square(i);
        }

        function add() {
            vm.output = calcService.add(vm.operand1, vm.operand2);
            setPropertiesVisible(false, false);
        }

        function subtract() {
            vm.output = calcService.subtract(vm.operand1, vm.operand2);
            setPropertiesVisible(false, false);
        }

        function divide() {
            vm.output = calcService.divide(vm.operand1, vm.operand2);
            setPropertiesVisible(false, false);
        }

        function multiply() {
            vm.output = calcService.multiply(vm.operand1, vm.operand2);
            setPropertiesVisible(true, false);
        }

        function setPropertiesVisible(isMultiplicationTableVisible, isSquaresVisible) {
            vm.showMultiplicationTable = isMultiplicationTableVisible;
            vm.showSquares = isSquaresVisible;
        } 
    }

})();

edit

I didn't think everything through when I wrote the review. The controller still has a lot of similar code which can be encapsulated.

Here is a revised version of calcController.js

```
(function() {

angular.module('calculatorApp')
.controller('calcController', calcController);

calcController.$inject = ['calcService'];

function calcController(calcService) {
var vm = this,
operations = {};

vm.showMultiplicationTable = false;
vm.showSquares = false;
vm.output = null;
vm.result = null;
vm.k = [];
vm.operand1 = null;
vm.operand2 = null;

vm.squared = squared;
vm.square = square;

vm.performOperation = performOperation;

init();

function init() {

operations = {
add: calcService.add,
subtract: calcService.subtract,
divide: calcService.divide,
multiply: calcService.multiply
};

var arrayLength = 25,
k = [];

for (var i = 0; i < arrayLength; i++) k[i] = i;

vm.k = k;
}

function squared(number) {
vm.result = calcService.square(number);
setPropertiesVisible(false, true);
}

function square(i) {
return calcService.square(i);
}

function performOperation(operation, m, s) {

Code Snippets

(function(){
    angular.module('calculatorApp', []);
})();

(function(){

    angular.module('calculatorApp')
    .factory('calcService', calcService);

    function calcService() {
        var formulas = {
            square: square,
            multiply: multiply,
            add: add,
            subtract: subtract,
            divide: divide
        };

        return formulas;

        function square(a) {
            return formulas.multiply(a, a);
        }

        function multiply(a, b) {
            return a * b;
        }

        function add(a, b) {
            return a + b;
        }

        function subtract(a, b) {
            return a - b;
        }

        function divide(a, b) {
            return a / b;
        }

    }

})();

(function() {

    angular.module('calculatorApp')
    .controller('calcController', calcController);


    calcController.$inject = ['calcService'];

    function calcController(calcService) {
        var vm = this;

        vm.showMultiplicationTable = false;
        vm.showSquares = false;
        vm.output = null;
        vm.result = null;
        vm.k = [];
        vm.operand1 = null;
        vm.operand2 = null;

        vm.squared = squared;
        vm.square = square;
        vm.add = add;
        vm.subtract = subtract;
        vm.divide = divide;
        vm.multiply = multiply;

        init();


        function init() {
            console.log(vm.result);
            var arrayLength = 25,
                k = [];

            for (var i = 0; i < arrayLength; i++) k[i] = i;

            vm.k = k;   
        }

        function squared(number) {
            vm.result = calcService.square(number);
            setPropertiesVisible(false, true);
        }

        function square(i) {
            return calcService.square(i);
        }

        function add() {
            vm.output = calcService.add(vm.operand1, vm.operand2);
            setPropertiesVisible(false, false);
        }

        function subtract() {
            vm.output = calcService.subtract(vm.operand1, vm.operand2);
            setPropertiesVisible(false, false);
        }

        function divide() {
            vm.output = calcService.divide(vm.operand1, vm.operand2);
            setPropertiesVisible(false, false);
        }

        function multiply() {
            vm.output = calcService.multiply(vm.operand1, vm.operand2);
            setPropertiesVisible(true, false);
        }

        function setPropertiesVisible(isMultiplicationTableVisible, isSquaresVisible) {
            vm.showMultiplicationTable = isMultiplicationTableVisible;
            vm.showSquares = isSquaresVisible;
        } 
    }

})();

(function() {

    angular.module('calculatorApp')
    .controller('calcController', calcController);


    calcController.$inject = ['calcService'];

    function calcController(calcService) {
        var vm = this,
            operations = {};

        vm.showMultiplicationTable = false;
        vm.showSquares = false;
        vm.output = null;
        vm.result = null;
        vm.k = [];
        vm.operand1 = null;
        vm.operand2 = null;

        vm.squared = squared;
        vm.square = square;

        vm.performOperation = performOperation;

        init();


        function init() {

            operations = {
                add: calcService.add,
                subtract: calcService.subtract,
                divide: calcService.divide,
                multiply: calcService.multiply
            };

            var arrayLength = 25,
                k = [];

            for (var i = 0; i < arrayLength; i++) k[i] = i;

            vm.k = k;   
        }

        function squared(number) {
            vm.result = calcService.square(number);
            setPropertiesVisible(false, true);
        }

        function square(i) {
            return calcService.square(i);
        }

        function performOperation(operation, m, s) {
            var func = operations[operation];
            vm.output = func(vm.operand1, vm.operand2);

            setPropertiesVisible(m, s);
        }

        function setPropertiesVisible(isMultiplicationTableVisible, isSquaresVisible) {
            vm.showMultiplicationTable = isMultiplicationTableVisible;
            vm.showSquares = isSquaresVisible;
        } 
    }

})();

<button ng-click="c.performOperation('add', false, false)">Add</button>
<button ng-click="c.performOperation('subtract', false, false)">Subtract</button>
<button ng-click="c.performOperation('multiply', true, false)">Multiply</button>
<button ng-click="c.performOperation('divide', false, false)">Divide</button>

Context

StackExchange Code Review Q#123568, answer score: 4

Revisions (0)

No revisions yet.