Check if before running for loop - javascript

Question

In Typescript (ES6), I have a queue running on an interval of 1 ms and I want to know which approach is better practice for performance.

1.

setInterval(() => {
  //if (this._queue.filter(a => !a.running && a.cbs.length) { // incorrect
  if (this._queue.filter(a => !a.running && a.cbs.length).length) { //edit
    for (let i = 0; i < this._queue.filter(a => !a.running && a.cbs.length).length; i++) {
      ...
    }
  }
}, 1);

setInterval(() => {
  for (let i = 0; i < this._queue.filter(a => !a.running && a.cbs.length).length; i++) {
    ...
  }
}, 1);

In approach #1, it obviously has an extra line of code, but I am pretty sure the if would take less CPU computation on each iteration of interval. Is this correct?

In approach #2, it has to define i and then run the filter and then attempt to iterate.

This may be such a low difference in performance it may not matter, but I am interested nonetheless.

True, if it passes the if on approach 1. But what if it only passes the if seldomly? — user1779362
– user1779362, Commented Nov 23, 2020 at 12:19

Medet Tleukabiluly · Accepted Answer · 2020-11-23 12:22:29Z

3

Your if statement is incorrect

if (this._queue.filter(a => !a.running && a.cbs.length) {
// this always resolves as true
// should be
if (this._queue.filter(a => !a.running && a.cbs.length).length) {

Just reuse the iteration

setInterval(() => {
  const arr = this._queue.filter(a => !a.running && a.cbs.length)
  if (arr.length) {
    for (let i = 0; i < arr.length; i++) {
      ...
    }
  }
}, 1);

answered Nov 23, 2020 at 12:22

Medet Tleukabiluly

12k3 gold badges38 silver badges78 bronze badges

Sign up to request clarification or add additional context in comments.

Comments

Guerric P · Accepted Answer · 2020-11-23 12:22:42Z

3

In both cases you are evaluating the filter expression on each iteration. The best thing you could do is evaluate the filter just once:

setInterval(() => {
  const queueLength = this._queue.filter(a => !a.running && a.cbs.length).length;
  for (let i = 0; i < queueLength;  i++) {
    ...
  }
}, 1);

answered Nov 23, 2020 at 12:22

Guerric P

32k6 gold badges61 silver badges109 bronze badges

Comments

habib chabbi · Accepted Answer · 2020-11-23 12:32:22Z

-1

pay attention to make sure that you evaluate the value of the filter outside the for : Otherwise you will be evaluating the value of the filter with each iteration and even if does not depend on the iteration i:

you can verify this by adding a console log inside the filter iteretee.

setInterval(() => {
 for (let i = 0; i < this._queue.filter(a => {console.log(a); reurn !a.running && 
 a.cbs.length} ).length; i++) 
 {
 ...
 }
 }, 1);

Sou it would be better to define a constant having the value this._queue.filter(a => !a.running && a.cbs.length) outside the loop

answered Nov 23, 2020 at 12:32

habib chabbi

984 bronze badges

2 Comments

user1779362 Over a year ago

I honestly have never thought about this, very good point!

Ilijanovic Over a year ago

this has a bad performance. it runs the .filter() method after ever iteration

Collectives™ on Stack Overflow

Check if before running for loop - javascript

3 Answers 3

Comments

Comments

2 Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

3 Answers 3

Comments

Comments

2 Comments

Your Answer

Sign up or log in

Post as a guest

Related