Beginner's Calculator code

Question

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.showSquares = false;
  }
  $scope.divide = function() {
    this.output = calcService.divide(this.operand1, this.operand2);
    $scope.showMultiplicationTable = false;
    $scope.showSquares = false;
  }

  $scope.multiply = function() {
    this.output = calcService.multiply(this.operand1, this.operand2);
    $scope.showMultiplicationTable = true;
    $scope.showSquares = false;
  }

  $scope.square = function(i) {
    return calcService.square(i);
  }

  var arrayLength = 25;
  var k = new Array(arrayLength);
  for (var i = 0; i < arrayLength; i++) {
    k[i] = i;
  }
  $scope.k = k;
});

<script src="https://ajax.googleapis.com/ajax/libs/angularjs/1.2.23/angular.min.js"></script>

<body ng-app="calculatorApp">
  <h1>AngularJS - Calculator</h1>
  <div ng-controller="calcController as calulator">
    <label>Enter a number:</label>
    <input type="number" ng-model="number" />
    <button ng-click="squared()">X<sup>2</sup>
    </button>
    <p>Result: {{result}}</p>
    <br />
    <br />
    <label>Enter a number:</label>
    <input type="number" ng-model="operand1" />
    <br />
    <label>Enter a second number:</label>
    <input type="number" ng-model="operand2" />
    <br />
    <button ng-click="add()">Add</button>
    <button ng-click="subtract()">Subtract</button>
    <button ng-click="multiply()">Multiply</button>
    <button ng-click="divide()">Divide</button>
    <p>Result: {{ output }}</p>

    <table ng-show="showMultiplicationTable">
      <tr>
        <th ng-repeat="item in k">{{item+1}}</th>
      </tr>
      <tr ng-repeat="item in k">
        <td ng-repeat="l in k">
          {{(l + 1) * (item + 1)}}
        </td>
      </tr>
    </table>

    <table ng-show="showSquares">
      <tr>
        <th>intial</th>
        <th>square</th>
      </tr>
      <tr ng-repeat="i in k">
        <td>{{ i + 1 }}</td>
        <td>{{square(i + 1)}}</td>
      </tr>
    </table>
  </div>
</body>

Just a little tip. Try to use "controller as" syntax as opposed to $scope More info here: toddmotto.com/digging-into-angulars-controller-as-syntax — Nilzone-, Mar 23 '16 at 11:34
@Nilzone-, that is a very good suggestion. you should post a review talking about that in your own words, I would definitely up-vote it! — Malachi, Mar 23 '16 at 12:07
I updated some of the code a little bit, if you are interested. — Nilzone-, Mar 27 '16 at 18:28

Nilzone- · Accepted Answer · 2016-03-27 18:27:44Z

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) {
            var func = operations[operation];
            vm.output = func(vm.operand1, vm.operand2);

            setPropertiesVisible(m, s);
        }

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

})();

and in the HTML code when the new function is used, it looks like this:

<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>

thank you for taking the time to write out a review, I appreciate the links as well — Malachi, Mar 23 '16 at 16:42
Could you explain the justification behind recommending the OP use an init() function? Controllers (through controller as) are constructors and should return a ready-to-use object without requiring any other function calls. — Dan, Mar 23 '16 at 16:44
@DanPantry just the fact that you avoid having code in the controller which isn't tied to any property or function. To me it just makes everything more readable. — Nilzone-, Mar 23 '16 at 16:54
Instead of console.log you can try and use the $log service of Angular, as well — A. Romeu, Mar 24 '16 at 8:17

A. Romeu · Accepted Answer · 2016-03-23 09:47:14Z

some comments on your code

First, you are adding an intermediate layer which is not really doing anything. calcService does not serve any purpose in your code, you can call directly MathService from the controller, so you could remove it.

You can also call service functions from html, if you assign the service to the controller scope, something like $scope.calcService = calcService

Then in the html you can directly hit {{calcService.square(i + 1)}}

Some logic can also be extracted for hiding and displaying results. A method can contain initialization to false for showing tables like this

$scope.initializeTable = function() {
    $scope.showMultiplicationTable = false;
    $scope.showSquares = false;
}

Then on each method you call it at the beginning and later you display the one that you want

$scope.multiply = function() {
    $scope.initializeTable;
    this.output = calcService.multiply(this.operand1, this.operand2);
    $scope.showMultiplicationTable = true;
}

Finally, maybe you would like to give a more significant naming to variable k, which it does not mean much right now

Have fun!

Stack Exchange Network

current community

your communities

more stack exchange communities

Beginner's Calculator code

2 Answers 2

Your Answer

Not the answer you're looking for? Browse other questions tagged javascript beginner html angular.js calculator or ask your own question.

Hot Network Questions

Beginner's Calculator code

2 Answers 2

Your Answer

Sign up or log in

Post as a guest

Not the answer you're looking for? Browse other questions tagged javascript beginner html angular.js calculator or ask your own question.

Related

Hot Network Questions