0

Imagine I have a list of objects that are Questions. I have to find if they are visible and if they are visible I have to change their answer to visible as well. (The example may not be practical it is just an example) If this is no side effect way (is it?):

questions.filter(function(question) {
    return question.isVisible;
})
.map(function(visibleQuestion) {
    return getAnswer(visibleQuestion);
})
.map(function(answer) {
    answer.isVisible = true;
});

and this is side effect way:

questions.forEach(function(question) {
    if (question.isVisible) {
        var answer = getAnswer(visibleQuestion);
        answer.isVisible = true;
    }
});

Why choose the no side effect way if you have to iterate 3 times to do your job?

PS. I could be wrong of what is side effect and what is not and what really would be the two ways of handling this.

h3dkandi
  • 1,106
  • 1
  • 12
  • 27
  • 2
    I think that if you were using purely side-effect-free code, you wouldn't return the original `answer`; you would return a copy of the original answer but with `isVisible` set to `true`. In this case, it doesn't make sense to go through hoops to avoid side effects. – Zev Spitz Mar 14 '17 at 12:29
  • 1
    And if JS's `map` and `filter` are lazy, you're actually only iterating once. – Carcigenicate Mar 14 '17 at 12:55
  • And as Zev mentioned, you're carrying out side effects anyways in your second `map`, so you're forcing functional style but adhering to imperative practices. The 2 don't mix well. – Carcigenicate Mar 14 '17 at 12:57

1 Answers1

0

The argument against callbacks and side effects is about maintainability and reclaiming the call stack so we can use keywords like throw and return.

The example you provided certainly makes it seem like an easy engineering trade off between readability and efficiency. That being said using a promise library you can achieve the same results without a callback/side effect all in a single loop.

promise.all(questions.map(function(question) {
        if (question.isVisible) {
            resolve(getAnswer(question));
        }
    }).then(function (arResults) {
        arResults.forEach(function(answer) {
            answer.isVisible = true;
            console.log(answer)
        });
    });
HopAlongPolly
  • 1,347
  • 1
  • 20
  • 48