Looping Array object with conditions and comparison the performance with ECMA VS Vanilla JS

Question

The Plain vanilla JS needs to converted in to advanced ECMA. The below 3 snippets shown yields the correct result. Need to identify which among them is performance oriented

this.allUserDetails.forEach((el, indx) => {
 if (type === 0 && flagResumeAll === false) {
 if (el.extensionTime === time && el.userProctorSignal === 0) {
 this.toggleButtonDisplayType = 0;
 } else {
 flagResumeAll = true;
 }
 } else if (type === 1 && flagPauseAll === false) {
 if (el.extensionTime === time && el.userProctorSignal === 1) {
 this.toggleButtonDisplayType = 1;
 } else {
 flagPauseAll = true;
 }
 }
});

The above code could be repaced with Code I and Code II

Code I

if (type === 0 && flagResumeAll === false) { 
 this.allUserDetails.filter(el => {
 if(el.extensionTime === time && el.userProctorSignal === 0) {
 this.toggleButtonDisplayType = 0;
 } else {
 flagResumeAll = true;
 } 
 }) 
} else if (type === 1 && flagPauseAll === false) {
 this.allUserDetails.filter(el => {
 if(el.extensionTime === time && el.userProctorSignal === 1) {
 this.toggleButtonDisplayType = 1;
 } else {
 flagPauseAll = true;
 } 
}) 

Code II

this.allUserDetails.filter(el => {
 if(type === 0 && flagResumeAll === false && el.extensionTime === time && el.userProctorSignal === 0) {
 this.toggleButtonDisplayType = 0;
 } else if (type === 0 && flagResumeAll === false){
 flagResumeAll = true;
 } else if (type === 1 && flagPauseAll === false && el.extensionTime === time && el.userProctorSignal === 1) {
 this.toggleButtonDisplayType = 1;
 } else if(type === 1 && flagPauseAll === false) {
 flagPauseAll = true;
 } 
}) 

Which among them is yields better in performance replacing from plain/vanilla JS to Code I/II. Will this be further simplified..

• Ted Brownlow: is flagResumeAll used elsewhere in the code?

Answer 1

Array.some

In the loop when flagResumeAll becomes true nothing more will change thus it is best to exit the loop.

If flagResumeAll is already true then nothing can be done inside the loop so you should not even start it. The same with type if not 0 or 1 then the loop will do nothing.

Using the first example as a guide to what your code should do you can use the Array function Array.some to set flagResumeAll

Array.some will exit the iteration when "some" of the items returns true, Or think of it as "if any" of the items returns true (JavaScript's built in bad naming some implies more than one)

Array.some exits when the callback returns true and returns true, or when there is no match it iterates all items then returns false.

Rewrite

As a function as there is no context at all to guide naming. So the function is called doThing

const doThing = (type, time) => {
 if (type === 0 || type === 1) {
 return this.allUserDetails.some(el => {
 if (el.extensionTime === time && el.userProctorSignal === type) {
 this.toggleButtonDisplayType = type;
 } else {
 return true;
 }
 });
 }
 return false;
}
flagResumeAll = flagResumeAll || doThing(type, time);

Answer 2

It looks like the main difference between your original code snippet and the two revisions is that you're using .filter() instead of .forEach() - both of which came out around the same time, and both of which came out a while ago (IE 9 supports both).

It looks like you aren't doing any significant changes to the code that would improve speed, so you will only see marginal improvements to performance, if any. But, you can always benchmark each of them and find out for yourself how fast each version is.

The original solution is actually the best one among the three shown. There seems to be a big misunderstanding about what the filter() function is and how to use it. filter() is supposed to take an array and return a new array with some values filtered out. The values that get filtered out is decided by the function passed in. For example, if you want the even numbers in a list of numbers, you could do this:

const numbers = [1, 3, 4, 6, 7, 9]
const isEven = n => n % 2 === 0
console.log(numbers.filter(isEven)) // [ 4, 6 ]

You are not using filter() this way - you're just using is like an alternative forEach() function, which is a big no-no. You can just change the .filter() for .forEach() in each of your revisions. Once you have done that, both your original code and your first revision would work just fine. I wouldn't go with the second one though, there's too much inside the if conditions.

I don't really have any newer javascript feature to suggest for these code snippets, except that I generally recommend using the newer for of loop instead of forEach() - it's more powerful than .forEach(), and it fixes the issues that for-in loops had.