2
\$\begingroup\$

There is a simple form which takes two numbers num1, num2 and an operation op. After submission, a new object is created. I'm new to OOJS and any correction and improvements are appreciated. Is this the best way to code this small calculator in Javascript using Object Oriented approach?

window.onload = function() {

  let form = document.forms['cal-form'];
  form.addEventListener('submit', calculate);

  function calculate(e) {
    //prevent default form submission
    e.preventDefault();
    //get form values
    let num1 = parseInt(document.getElementsByTagName('input')[0].value);
    let num2 = parseInt(document.getElementsByTagName('input')[1].value);
    let op = document.getElementsByTagName('select')[0].value;

    //create object constructor function
    function Calculate(num1, num2, op){
      this.num1 = num1;
      this.num2 = num2;
      this.op = op;
    } 

    Calculate.prototype.result = function() {
      let res;
      switch (op) {
        case 'add':
          res = this.num1 + this.num2;
          break;
        case 'sub':
          res = this.num1 - this.num2;
          break;
        case 'mul':
          res = this.num1 * this.num2;
          break;
        case 'div':
          res = this.num1 / this.num2;
          break;
        default:
          res= 'Error! No operation selected.';
      }
      return res;
    };

    //create an object
    let cal = new Calculate(num1, num2, op);
    //display result
    document.getElementById('result').innerHTML = cal.result();

  }
};
\$\endgroup\$
1
\$\begingroup\$

If you want to use object-oriented javascript for this calculator, I would prefer the class syntax over the prototype syntax.

This prototype approach ..

function Calculate(num1, num2, op) {
      this.num1 = num1;
      this.num2 = num2;
      this.op = op;
    } 

    Calculate.prototype.result = function() {
       // ..
    }

.. can be replaced by (Fiddle):

class BinaryExpression {
    constructor(a, b, op) {
        this.a = a;
        this.b = b;
        this.op = op;
    }
    evaluate() {
        switch (this.op) {
          case 'add':
            return this.a + this.b;
          case 'sub':
            return this.a - this.b;
          case 'mul':
            return this.a * this.b;
          case 'div':
            return this.a / this.b;
          default:
            return 'Error! No operation selected.';
        }
    }
}

Misc

  • Most math expression compilers would implement a switch for the specific binary operators, but you could take it a step further and sub-class each binary operation.
  • Rather than returning a default message when the operator is not known, you could throw an exception and let the consumer handle it.
\$\endgroup\$
1
\$\begingroup\$

I don't really see any purpose to forcibly using a class setup here. All that's doing is allowing you to provide the data before result is called. That's not necessary here though, and there are other ways of achieving that. If you needed to create multiple Calculate objects, and retain them, and call result on them multiple times, there may be a purpose. I can't see any gain here though.

I would just collapse everything down to a function that accepts the three bits of data (op, num1 and num2) and just call the function directly when needed.


There's cleaner ways of handling the "dispatching" than a switch. I'd just use a regular JavaScript object here:

// Associate the operator strings
//  with functions
var strToOp = {"+": (x, y) => x + y, 
               "-": (x, y) => x - y,   
               "*": (x, y) => x * y,
               "/": (x, y) => x / y};

var num1 = 2;
var num2 = 5;
var op = "*";

// Get a function from the map
var func = strToOp[op];

// func will be undefined if they supply a bad operator string
// This is roughly equivalent to your "default" case
if(func) {
    // And give the numbers to it
    var result = func(num1, num2);

    // Prints 10
    console.log(result);

} else {
    // Handle bad operator
} 
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.