JS calculator, respecting order of operations

Question

I have a function that receives an array of numbers and operations

Ex: userEntry = [3,+,3,x,3] -> returns 12

It is a calculator that adheres to the proper order of operations, therefore multiply/divide behaves differently than addition/subtraction. I have one redundant line of code:

userEntry = calculatorOperations
 .calculationSequence(operationsMD[i], indexOfOperand, userEntry);

is similar to:

 userEntry = calculatorOperations
 .calculationSequence(userEntry[1], indexOfOperand, userEntry);

With different behavior between operations, is it possible to remove this redundant line or is it neccesary?

function operateOnEntry(userEntry) {
//this is where the calculations occur when hitting =
const operationsMD = ['x', '/'];
let indexOfOperand;
let operation;
while (userEntry.includes('x') || userEntry.includes('/')) {
 let i = 0;
 if (!userEntry.includes('x')) {
 i++;
 }
 indexOfOperand = userEntry.indexOf(operationsMD[i]);
 userEntry = calculatorOperations
 .calculationSequence(operationsMD[i], indexOfOperand, userEntry);
}
while (userEntry.includes('+') || userEntry.includes('-')) {
 indexOfOperand = 1;
 userEntry = calculatorOperations
 .calculationSequence(userEntry[1], indexOfOperand, userEntry);
}
return userEntry;
}
let calculatorOperations = {
'x': (arg1, arg2) => {
 return arg1 * arg2;
},
'/': (arg1, arg2) => {
 return arg1 / arg2;
},
'+': (arg1, arg2) => {
 return arg1 + arg2;
},
'-': (arg1, arg2) => {
 return arg1 - arg2;
},
returnIndexOfEntry: (index, userEntry) => {
 let arg1 = Number(userEntry[index - 1]);
 let arg2 = Number(userEntry[index + 1]);
 return [arg1, arg2];
},
returnSpliced: (index, newTotal, userEntry) => {
 userEntry.splice((index - 1), 3, newTotal);
 return userEntry;
},
calculationSequence: (operation, indexOfOperand, userEntry) => {
 let getArgs = calculatorOperations.returnIndexOfEntry(indexOfOperand, userEntry);
 let newTotalForEntry = calculatorOperations[operation](getArgs[0], getArgs[1]);
 let newUserEntry = calculatorOperations.returnSpliced(indexOfOperand, newTotalForEntry, userEntry);
 return newUserEntry;
}
};

• radarbob: operationsMD seems superfluous. If a x or / is found in userEntry, well there it is. Why have a separate array for these operators - and then only these operators.
• radarbob: is it possible to remove this redundant line Not as long as there are separate loops for multiplication and addition. Perhaps extract a while loop to it's own function and pass the operator set you're looking for. That is pass operationsMD = ['x', '/'] and then pass operationsMD = ['+', '-']
• Sᴀᴍ Onᴇᴌᴀ: @radarbob Please don't edit the code just to adjust whitespace. Please refer to answers to this meta post for reasoning.
• radarbob: @SᴀᴍOnᴇᴌᴀ, good point given this is Code Review. FWIW indenting misalignment is most often the annoying nature of writing code blocks here. As for moving that if to a 1-liner, I beg mercy from the court.
• FreezePhoenix: @radarbob what court...?

Answer 1

Your Question

With different behavior between operations, is it possible to remove this redundant line or is it neccesary?

Yes you can iterate over the operations using Object.keys(calculatorOperations), since the functions are listed in order of highest precedence first:

Object.keys(calculatorOperations).forEach(function(functionName) {
 while(userEntry.includes(functionName)) {
 indexOfOperand = userEntry.indexOf(functionName);
 userEntry = calculatorOperations
 .calculationSequence(functionName, indexOfOperand, userEntry);
 }
}); 

Though be aware that all keys will be iterated over, including returnIndexOfEntry, returnSpliced and calculationSequence. If you didn't want those to be iterated over, the functions for the four mathematical operations could be moved into a sub-property and those could be iterated over instead, or else move the other helper functions out of the object.

With this approach, there is no need for the variable operationsMD.

Other feedback

• As a default, use const for variables. When you determine that it needs to be re-assigned (especially for loop interators/counters) then use let. So variables like calculatorOperations, getArgs, newUserEntry, etc. can be declared with const

• As @FreezePhoenix mentioned, arrow functions that only have one line in the body don't need to be surrounded by braces. For example, the calculation for times:

  'x': (arg1, arg2) => {
   return arg1 * arg2;
  },
  

  Can be simplified to:

  'x': (arg1, arg2) => arg1 * arg2,
  

• Presumably the inconsistencies with the indentation are due to the pasting here but make sure you always use the same amount of indentation (e.g. two or four spaces).

Updated code

See code updated per recommendations below.

function operateOnEntry(userEntry) {
 let indexOfOperand;
 let operation;
 Object.keys(calculatorOperations).forEach(function(functionName) {
 while (userEntry.includes(functionName)) {
 indexOfOperand = userEntry.indexOf(functionName);
 userEntry = calculationSequence(functionName, indexOfOperand, userEntry);
 }
 });
 return userEntry;
}
const returnIndexOfEntry = (index, userEntry) => {
 const arg1 = Number(userEntry[index - 1]);
 const arg2 = Number(userEntry[index + 1]);
 return [arg1, arg2];
};
const returnSpliced = (index, newTotal, userEntry) => {
 userEntry.splice((index - 1), 3, newTotal);
 return userEntry;
};
const calculationSequence = (operation, indexOfOperand, userEntry) => {
 const getArgs = returnIndexOfEntry(indexOfOperand, userEntry);
 const newTotalForEntry = calculatorOperations[operation](getArgs[0], getArgs[1]);
 const newUserEntry = returnSpliced(indexOfOperand, newTotalForEntry, userEntry);
 return newUserEntry;
}
const calculatorOperations = {
 'x': (arg1, arg2) => arg1 * arg2,
 '/': (arg1, arg2) => arg1 / arg2,
 '+': (arg1, arg2) => arg1 + arg2,
 '-': (arg1, arg2) => arg1 - arg2
};
var userEntry = [3, '+', 3, 'x', 3];
console.log(operateOnEntry(userEntry));

• FreezePhoenix: Am I allowed to edit my answer in response to yours?...
• Sᴀᴍ Onᴇᴌᴀ: I think so- maybe just give credit, if you don’t mind ;)
• FreezePhoenix: Will this do: codereview.stackexchange.com/posts/204864/revisions?
• Sᴀᴍ Onᴇᴌᴀ: Yes- "Great minds..."
• FreezePhoenix: "... think alike." ;)

Answer 2

Overview

• If you are using ES6, you should use the full ability of arrow functions: (args) => returnVal
• In addition, some of the methods don't need to be defined as a property.
• calculatorOperations should be a constant.
• You don't need to access each element in an array like this: (getArgs[0], getArgs[1]), you can do (...getArgs)
• I agree with @SamOnela in that you can iterate over the operations. However, their method pollutes global namespace

Questions

• Why are you using x not *?

Rewrite

 function operateOnEntry(userEntry) {
 let indexOfOperand,
 operation;
 Object.keys(calculatorOperations.ops).forEach(function(functionName) {
 while (userEntry.includes(functionName)) {
 indexOfOperand = userEntry.indexOf(functionName);
 userEntry = calculatorOperations.calculationSequence(functionName, indexOfOperand, userEntry);
 };
 });
 return userEntry;
 }
 
 const calculatorOperations = {
 ops: {
 'x': (arg1, arg2) => arg1 * arg2,
 '/': (arg1, arg2) => arg1 / arg2,
 '+': (arg1, arg2) => arg1 + arg2,
 '-': (arg1, arg2) => arg1 - arg2
 },
 returnIndexOfEntry(index, userEntry) {
 let arg1 = Number(userEntry[index - 1]),
 arg2 = Number(userEntry[index + 1]);
 return [arg1, arg2];
 },
 returnSpliced(index, newTotal, userEntry) {
 userEntry.splice((index - 1), 3, newTotal);
 return userEntry;
 },
 calculationSequence(operation, indexOfOperand, userEntry) {
 
 let getArgs = calculatorOperations.returnIndexOfEntry(indexOfOperand, userEntry),
 newTotalForEntry = calculatorOperations.ops[operation](...getArgs),
 newUserEntry = calculatorOperations.returnSpliced(indexOfOperand, newTotalForEntry, userEntry);
 return newUserEntry;
 }
 };
 var userEntry = [3, '+', 3, 'x', 3];
 console.log(operateOnEntry(userEntry));