1
\$\begingroup\$

Is there a better way to loop item of an array using forEach and make repeat it using setTimout/Interval or something else?

I need that each item of the array was repeated each value of delay for some time (duration):

const arr = [
  { delay: 1_000, duration: 3_000, msg: "Hello World" }, //repeat 3 times msg for 3 sec
  { delay: 2_000, duration: 4_000, msg: "Sup?" } //repeat 2 times msg for 3 sec
];

Here is the link and here is the code but I think it's a difficult solution:

const arr = [
  { delay: 1_000, duration: 3_000, msg: "Hello World" }, //repeat msg each sec for 3 sec
  { delay: 2_000, duration: 4_000, msg: "Sup?" } //repeat msg each 2 sec for 4 sec
];

class LoopElem {
  constructor(configs) {
    this.count = 0;
    this.sumDelay = 0;
    this.toLoopElem(configs);
  }

  toLoopElem = (configs) => {
    const that = this;
    const { delay, duration, msg } = configs[that.count];
    setTimeout(function request() {
      that.sumDelay += delay;
      console.log(msg); // each delay shows msg
      if (that.sumDelay >= duration) {
        that.sumDelay = 0;
        that.count += 1;
        // if array have already no item stop repeat it
        if (!configs[that.count]) {
          return null;
        }
        setTimeout(that.toLoopElem, configs[that.count].delay, configs);
        return null;
      }
      setTimeout(request, delay);
    }, delay);
  };
}

new LoopElem(arr);

\$\endgroup\$
2
\$\begingroup\$

As you suspect the code can be improved by making it less complex and convoluted. As posted the LoopElem class avoids creating variables in global scope by using instance object properties and function scoped variables in the toLoopElem class method.

The option to avoid creating global variables by rewriting the code using nested functions without using JavaScript classes has been covered in BlindMan67's excellent answer which I won't revisit here.

What I want to bring up is that if the LoopElem class needs to have additional methods added to it, say to stop, rewind, pause or fast forward messages as you would in a media player, there is a case for using a class definition. However, you would call a message play method for outside code, not from within the constructor.

Two points about the question first:

  1. Array.prototype.forEach is a synchronous method which calls its function argument parameters in quick succession for each array entry - you can't delay calls between entries. So it won't be useful for this case.

  2. setInterval is no more accurate that setTimeout and internally uses setTimeout code without creating a new timer id to return to the caller. Since different messages use different delays between repeats, using setInterval could make the code more rather than less complex.

Here's a LoopBox class example that creates a message player with two methods: play and stop. The problem of this vs that usage is solved by using an arrow function for repeat to access object instance this values, and making playLoop an ordinary function instead of an arrow function.

class LoopBox {
  constructor(loops) {
    this.loops = loops;
    this.loopIndex = -1;
    this.timerId = 0;
  }
  play() {
    if( ++this.loopIndex < this.loops.length) {
      this.playLoop( this.loops[ this.loopIndex ]);
    }
    else {
      this.stop();
    }
  }
  playLoop(loop) { 
      let sumDelay = 0;  
      const repeat = () => {
        console.log( loop.msg);
        sumDelay += loop.delay;
        if( sumDelay >= loop.duration) {
          this.play();  
        }
        else {
          this.timerId = setTimeout( repeat, loop.delay);
        }
      }
      this.timerId = setTimeout( repeat, loop.delay);
  }
  stop() {
      clearTimeout( this.timerId);
      this.loopIndex = -1;
  }
}
const looper = new LoopBox( [
  { delay: 1_000, duration: 3_000, msg: "Hello World" }, //repeat msg each sec for 3 sec
  { delay: 2_000, duration: 4_000, msg: "Sup?" } //repeat msg each 2 sec for 4 sec
]);

looper.play();

\$\endgroup\$
2
\$\begingroup\$

Source code is too complex

The code is way too complex for what you are doing Mainly due to all of the superfluous code.

Not needed

  • all the return null,
  • all of the class code for object loopElm
  • that = this (you should never need to do that!!)

and with some odd naming it is very hard to decipher your intent

Rewrite

The following does the same but without all the additional source code complexity.

I changed the duration value to repeat. This value can be calculated from duration / delay but I don't see the point

function timeEventsCycle(timing) {
    var count = 0;        
    next();
    function next(info) {
        count = count <= 0 ? (info = timing.shift())?.repeat - 1 : count - 1;
        if (info) {
            console.log(info.msg)
            setTimeout(next, info.delay, info);
        }
    }
}

As timeout can be very inaccurate you can track the time and adjust the timeout to keep as close as possible to the timing you want.

The next rewrite keeps the times as close a possible to the requested times by adjusting the timeout to speed up if falling behind schedule.

Updated and fixed version

function timeEventsCycle(timing) {
    const getCount = info => info && (info.duration / info.delay) - 1;
    var count = -1;    
    (function next(info) {
        count >= 0 && console.log(info.msg );
        count = count <= 0 ? getCount(info = timing.shift()) : count - 1;
        info && setTimeout(next, info.delay, info);
    })();
}

console.log("start")
timeEventsCycle([
  { delay: 1000, duration: 3000, msg: "Hello World" }, 
  { delay: 2000, duration: 4000, msg: "Sup?" } 
]);

NOTE: the log output "start" is only to show the delay between starting and first message.

\$\endgroup\$
3
  • \$\begingroup\$ - all of the class code for object loopElm So? It has to be a class, it's a condition -that = this (you should never need to do that!!) Why? 1. First "Sup" shows in 1 sec but should be in 2 sec 2. We can't change the data \$\endgroup\$
    – HackerMF
    Jun 14 at 6:08
  • \$\begingroup\$ Use Function.bind rather than that = this eg the timeout callback setTimeout(function request() { /* code */ }, becomes setTimeout((function request() { /* code */ }).bind(this), \$\endgroup\$
    – Blindman67
    Jun 14 at 9:47
  • \$\begingroup\$ got it, thank you \$\endgroup\$
    – HackerMF
    Jun 15 at 8:05

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.