Array.reduce mutating state outside is not recommended?

Question

Why is mutating variables outside .reduce() method considered bad practice? In the example below I am mutating a variable declared outside from inside the reduce method. Why is it not recommended to do so?

function balanceParens(string) {
    let max = 0;
    let res = string.split("").reduce((counter, char) => {
        // Handle if parens open and close out of order
        if (counter < 0) { 
          return counter;
        }
        // Add 1 for each open in order
        if (char === "(") {
          if(++counter > max) {
            max = counter;
          }
          return counter;
        }
        // subtract 1 for each close in order
        if (char === ")") {
          return --counter;
        }
        // handle use case if char is not a paren
        return counter;
    }, 0);
    console.log("Max depth was :", max);
    return !res;
}
console.log(balanceParens("((()(((())))))((((()))))"));

If you do that, why bother using `reduce` at all? If your output is via side-effects you might as well be using `forEach`. — Jordan Running, Jul 06 '17 at 18:54
My question is not around whether I should use `forEach` or `reduce`. I simply want to know why is it bad to have side-effects. — Aftab Khan, Jul 06 '17 at 18:57
Related: https://softwareengineering.stackexchange.com/questions/15269/why-are-side-effects-considered-evil-in-functional-programming https://stackoverflow.com/questions/763835/are-side-effects-a-good-thing — Jordan Running, Jul 06 '17 at 18:58
Please consider deleting this question, as it's not useful for future StackOverflow readers. — Cory Petosky, Jul 07 '17 at 03:22

score 0 · Answer 1 · answered Jul 06 '17 at 18:56

See @JordanRunning comment:

"If you do that, why bother using reduce at all? If your output is via side-effects you might as well be using forEach"

I was about to say the same. And here is a version of the function without an external dependency:

function balanceParens(string) {
 let res = string.split("").reduce((state, char) => {
  // Handle if parens open and close out of order
  if (state.counter < 0) {
   return state;
  }
  // Add 1 for each open in order
  if (char === "(") {
   if (++state.counter > state.max) {
    state.max = state.counter;
   }
   return state;
  }
  // subtract 1 for each close in order
  if (char === ")") {
   state.counter = --state.counter;
   return state;
  }
  // handle use case if char is not a paren
  return state;
 }, {max: 0, counter: 0});
 console.log("Max depth was :", res.max);
 return !res.counter;
}

console.log(balanceParens("((()(((())))))((((()))))"));

I know this can be achieved without having to use max. I wanted to know if there is any specific reason why I shouldn't. Here is the [link](https://stackoverflow.com/a/44956127/2876143) to solution which IMO is more cleaner — Aftab Khan, Jul 06 '17 at 18:59

score 0 · Accepted Answer · answered Jul 06 '17 at 19:48

Why is mutating variables outside .reduce() method considered bad practice?

Because you are mixing the functional with the imperative approach. Keep it to a single paradigm instead of confusing everyone.

You would go with

either reduce with a pure callback and no side effects

function step({max: oldMax, counter: oldCounter}, char) {
  const counter = oldCounter + (char=="(") - (char==")");
  const max = counter > oldMax ? counter : oldMax;
  return {max, counter};
}
function maxParensLevel(string) {
  assert(hasBalancedParens(string));
  const {max, counter} = string.split("").reduce(step, {max:0, counter:0});
  return max;
}

or a simple loop and two mutable variables

function maxParensLevel(string) {
  assert(hasBalancedParens(string));
  let max = 0;
  let counter = 0;
  for (const char of string.split("")) {
    if (char == "(")
      counter++;
    else if (char == ")")
      counter--;
    if (counter > max)
      max = counter;
  }
  return max;
}

but not both.

Array.reduce mutating state outside is not recommended?

2 Answers2

Linked