Is it bad practice to mutate an array within a reduce function?

I’m solving the “Intermediate Algorithm Scripting: Drop It” challenge.
The prompt is:
"Given the array arr , iterate through and remove each element starting from the first element (the 0 index) until the function func returns true when the iterated element is passed through it.

Then return the rest of the array once the condition is satisfied, otherwise, arr should be returned as an empty array."

My solution was to return:
arr.reduce((a,b,i) => func(b) ? arr.splice(i) : a.concat([]), []);

Basically, I broke out of “reduce” by splicing when func(b) is true. If it’s never true, this just returns an empty array. This works but it seems like a hack-y solution.

I based it on the first answer from:

I’m just wondering if what I did is bad practice. I was trying to avoid using for loops and do this purely functionally, but clearly I had to mutate the array by splicing it. Is it justifiable in this case, or is there a better functional solution to this problem based on my own solution?

It’s not really a typical use of reduce, but it’s not particularly problematic. If this showed up in a code review at the places I’ve worked, someone would probably suggest doing it differently because it’s a bit awkward to read.

1 Like

The whole point of reduce is to create something new without mutating the original array.

You do not have to mutate the original array when using reduce to solve this challenge. You can create another array with reduce.

1 Like

Sorry, I don’t think I expressed myself correctly. What I mean’t was that I clearly did that in my solution, not that I had to do it.

That’s clever, but yeah, imo it is bad practice. Mainly because it’s [highly] unexpected usage, so difficult to read. A reduce function would normally take a list of things of one type of thing, iterate through all of it and return a new thing of some type, not modify the original list. You’re negating the clarity of reduce by doing that, doubly so because you’re trying to break out of something that isn’t designed to be easy to break from. If you’re going to mutate, then just write a loop and use multiple lines for clarity, you don’t need to golf the code (plus the loop version is likely to be more efficient, if that is a concern at all).

2 Likes

I think the OP has already been answered, but I’m curious for my sake:

Assuming using reduce was a requirement (ie efficiency is somehow less important), what problems would you cite in this similar non-mutating version?

{
const func = (i) => i > 3;  // example predicate function
const arr = [1, 2, 3, 4, 5, 6, 7, 1, 2, 3];
const result = arr.reduce((acc, cur) => (acc.length || func(cur)) 
        ? acc.concat(cur) 
        : acc
, []);
}

edit: even in the thought experiment sense, reduce as a requirement sounds silly. feel free to ignore that in any advice you’re willing to offer me

The problem is that it’s difficult to understand what’s going on. YMMV but anything other than very basic logic inside reduce is normally very hard to read. Efficiency shouldn’t really be a concern really unless there’s some pressing reason it needs to be, it works fine and it’s plenty efficient for what it does.

Edit: this is completely daft, but re efficiency and speed for a mutable function, I think this would be the best possible

function dropElements(arr, func) {
  for (let i = 0; i < arr.length; i++) {
    if (!func(arr[i])) {
      continue;
    } else {
      arr.splice(0, i);
      break;
    }
  }
  return arr;
}

Or

function dropElements(arr, func) {
  arr.splice(0, arr.findIndex(func));
  return arr;
}

& with slice

function dropElements(arr, func) {
  return arr.slice(arr.findIndex(func));
}
1 Like

similar to what @DanCouper said, the best practice isnt to make it as short as possible but to make it as readable as possible. being able to quickly understand what is going is more important than you code actually working

Is there any good way to do this that doesnt involve loops? I was trying to do this purely functionally. perhaps a recursive solution?

Example recursive:

function dropIt(arr, func) {
  const [head, ...rest] = arr;
  if (func(head)) return arr;
  return dropIt(rest, func);
}

Also, this one from prev post has no (explicit) loops:

function dropIt(arr, func) {
  return arr.slice(arr.findIndex(func))
}
1 Like