JS calculator module using functional programming

Question

All of the code is self-explanatory and pretty much complete.

What the code does:

Calculates using onscreen numerics and operators
Validates any erroneous operations, such as 4 ** 8 or *9

Good features of the code:

The code has separate functions handling the validation, evaluation, etc.
It's much more modular

Recommended further improvements on the following basis:

Understandability
Modularity
Efficiency
Repetition

(function() {
  // private variable accessible by all the functions below
  var keys, input = "",
    result;

  function equate() {
    console.log('the equal button was pressed')

    result = eval(input);
    $('.screen').text(result);
  }

  function clear() {
    input = "";
    $('.screen').text(input);
    result = "";
    console.log('the clear button was pressed')
  }


  function isOperator(_input) {
    return (['+', '-', '*', '%', '/', '.'].indexOf(_input) >= 0)
  }

  function lastInputIsOperator() {
    return isOperator(input.charAt(input.length - 1))
  };


  // created a new module validate input and moved it out of expressInMath
  function validateInput(_input) {
    // prevents any operations like ++, -- etc 
    if (isOperator(_input)) {
      // check the last character input
      if (lastInputIsOperator(input)) {
        if (input.length < 1) {
          input = '';
        } else if (_input === '.' && input.indexOf('.') >= 0) {
          console.log('invalid');
        } else {
          input = input.slice(0, input.length - 1) + _input;
        }
        // avoids any operation of the kind 0.6.7.5 
      } else if (_input === '.' && input.indexOf('.') >= 0) {
        console.log('invalid');
      } else {
        if (input.length < 1) {
          input = '';
        } else {
          input += _input;
        }
      }
      console.log('Operation prohibited')
    } else {
      input += _input;
    }
  }

  function expressInMath(_input) {
    validateInput(_input);
    result = '';
    $('.screen').text(input);
  }

  // analyzes the keys pressed by the user
  function analyze() {

    //private vars
    let $this = $(this),
      _input = $this.text();

    // if clear button is pressed
    if (_input == "C") {
      clear();
    }

    // if equals to button is pressed
    else if (_input == "=") {
      equate();
    }

    // if key or operator button is pressed
    else {
      expressInMath(_input);
    }
  }

  // self invoking initializing function
  (function initialize() {
    $('#calculator span').on('click', analyze);
  }());
}())

/* Basic reset */

* {
  margin: 0;
  padding: 0;
  box-sizing: border-box;
  font: bold 14px Arial, sans-serif;
}

html {
  height: 100%;
  background: radial-gradient()circle, #fff, #ccc;
  background-size: cover;
}

#calculator {
  width: 375px;
  height: 340px;
  margin: 100px auto;
  padding: 20px 20px 9px;
  background: yellowgreen;
  border-radius: 3px;
  box-shadow: 0px 4px #009de4, 10px 15px 0px rgba(0, 0, 0, 0.2);
}


/* top portion */

.top span.clear {
  float: left;
}

.top .screen {
  height: 40px;
  width: 212px;
  float: right;
  background: rgba(0, 0, 0, 0.2);
  box-shadow: inset 0px 4px #009de4;
  padding: 0px 10px;
  /* typography */
  font-size: 17px;
  line-height: 40px;
  color: white;
  text-shadow: 1px 1px 2px rgba(0, 0, 0, 0.2);
  text-align: right
}


/* Clear floats */


/*.keys, .top {overflow: hidden};*/

.keys span,
.top span.clear {
  float: left;
  position: relative;
  top: 0;
  cursor: pointer;
  width: 66px;
  height: 40px;
  background: white;
  border-radius: 2px;
  box-shadow: 0px 4px rgba(0, 0, 0, 0.2);
  margin: 0 7px 11px 0;
  line-height: 40px;
  text-align: center;
  user-select: none;
}

.keys span.operator {
  background: #FFF0F5;
  margin-right: 0px;
}

.keys span.eval {
  background: #f1ff92;
  box-shadow: 0px 4px #9da853;
  color: #888e5f;
}

.top span.clear {
  background: #ff9fa8;
  box-shadow: 0px 4px #ff7c87;
  color: white;
}


/* hover effect */

.keys span:hover {
  background: #9c89f6;
  box-shadow: 0px 4px #6b54d3;
  color: white;
}

.keys span.eval:hover {
  background: #f1ff92;
  box-shadow: 0px 4px #9da853;
  color: #888e5f;
}

.top span.clear:hover {
  background: #ff9fa8;
  box-shadow: 0px 4px #ff7c87;
  color: white;
}


/* active state */

.keys span:active {
  box-shadow: 0px 4px #6b54d3;
  top: 4px;
}

.keys span.eval:active {
  box-shadow: 0px 4px #9da853;
  top: 4px;
}

.top span.clear:active {
  top: 4px;
  box-shadow: 0px 0px #d3545d;
}

<html>

<head>
  <link rel="stylesheet" href="/style.css">
</head>

<body>
  <div id="calculator">
    <div class="top">
      <span class="clear">C</span>
      <div class="screen">0</div>
    </div>
    <div class="keys">
      <span>7</span>
      <span>8</span>
      <span>9</span>
      <span>+</span>
      <span>4</span>
      <span>5</span>
      <span>6</span>
      <span>-</span>
      <span>1</span>
      <span>2</span>
      <span>3</span>
      <span>/</span>
      <span class="num">0</span>
      <span class="decimal">.</span>
      <span class="eval">=</span>
      <span>*</span>
    </div>
  </div>

  <script src="https://cdnjs.cloudflare.com/ajax/libs/jquery/2.1.3/jquery.min.js"></script>
  <script src="https://cdnjs.cloudflare.com/ajax/libs/lodash.js/4.17.4/lodash.min.js"></script>
  <script src="new.js"></script>
  <!-- <script src="script.js"></script> -->
</body>

</html>

janos · Accepted Answer · 2017-08-22 17:20:31Z

Possible bug

This part in validateInput looks like a bug to me:

if (isOperator(_input)) {
  // check the last character input
  if (lastInputIsOperator(input)) {
    // ...
  }
  console.log('Operation prohibited')

That is, when isOperator(_input) is true, the Operation prohibited message is logged. If that's intentional, I don't understand why.

Simplify

This complex chain of conditions can be simplified:

  // check the last character input
  if (lastInputIsOperator(input)) {
    if (input.length < 1) {
      input = '';
    } else if (_input === '.' && input.indexOf('.') >= 0) {
      console.log('invalid');
    } else {
      input = input.slice(0, input.length - 1) + _input;
    }
    // avoids any operation of the kind 0.6.7.5 
  } else if (_input === '.' && input.indexOf('.') >= 0) {
    console.log('invalid');
  } else {
    if (input.length < 1) {
      input = '';
    } else {
      input += _input;
    }
  }

Most notably, you check for multiple dots whether the last input is an operator or not, so that can be lifted out of the chain to appear only once.

Also, when input.length < 1, then input is already the empty string.

This is equivalent:

if (input.length > 0) {
  if (_input === '.' && input.indexOf('.') >= 0) {
    console.log('invalid');
  } else if (endsWithOperator(input)) {
    input = input.slice(0, input.length - 1) + _input;
  } else {
    input += _input;
  }
}

I also took the liberty and renamed lastInputIsOperator to endsWithOperator which doesn't repeat "input" which is implied by the function's parameter, and sounds like a very natural sentence.

Performance

The isOperator function creates a temporary array every time it's called. It would be better to store ['+', '-', '*', '%', '/', '.'] in a constant.

Also note that indexOf does a linear search. It would be better to emulate a hash table by storing the operators in an object.

As yet another alternative, when the operators are all a single character, you could have used a simple string to store them.

Stack Exchange Network

JS calculator module using functional programming

1 Answer 1

Possible bug

Simplify

Performance

You must log in to answer this question.

Hot Network Questions

JS calculator module using functional programming

1 Answer 1

Possible bug

Simplify

Performance

You must log in to answer this question.

Related

Hot Network Questions