2
\$\begingroup\$

Well this could seem redundant given the numerous topic about interaction between Promises and setTimeout but I can't find an answer that exactly fit my case (or that I am capable of interpreting !)

In the case of a Video player API usage, I need to loop on a sequence, and exit the loop when a certain condition is reached.

The following code works, but I was wondering if there was a sexier way, using Promise chaining instead of this :

export class LoopStep {
// looping from 0 to 5 seconds in the video
this.duration = 5; 
this.start = 0;

onStart() { // entry point
    return this.onLoop();
}

onLoop() {
    return new Promise((resolve) => {
        this.loopAndResolve(resolve);
    });
}

loopAndResolve(resolve) { // start loop
    if(this.isLooping) { // exit condition when turned to false
        this.player.setTime(this.start);
        this.player.play();
        setTimeout(this.loopAndResolve.bind(this, resolve), this.duration * 1000); // callback
    } else {
        resolve();
    }
}
}

// Anywhere else
loopStep.onStart.then(()=> console.log('exiting loop successfully')); // OK

Actually I would find more appropriated to avoid passing the "resolve" function as a parameter (only because I have a feeling that this could be wrong, or at least not "the most efficient/sexy").

\$\endgroup\$
  • \$\begingroup\$ You can simply embed loopAndResolve as a named function inside onLoop so there'll be no need to pass resolve. \$\endgroup\$ – wOxxOm Feb 7 '18 at 5:28
0
\$\begingroup\$

You could try something like this:

export class LoopStep {
    public onStart(): Promise<void> { // entry point
        return this.onLoop();
    }

    private onLoop(): Promise<void> {
        return this.loopAndResolve();
    }

    private loopAndResolve(): Promise<void> { // start loop
        if (this.isLooping) { // exit condition when turned to false
            this.player.setTime(this.start);
            this.player.play();
            setTimeout(() => this.loopAndResolve(), this.duration * 1000);
        } else {
            return Promise.resolve();
        }
    }
}

// Anywhere else
loopsStep.onStart().then(() => console.log('exiting loop successfully')); // OK

One of the best things of Typescript is the type safety. It's easier to keep track of the promises you're passing from one function to another if you declare them in the return type.

In your case, where you just want a cleaner way to resolve a Promise, you can use Promise.resolve();. It's pretty straightforward to use and very useful. DOCS

(Don't forget the parenthesis when calling onStart!)

\$\endgroup\$
  • \$\begingroup\$ I see your point. Nevertheless I'm not sure how to combine it with setTimeout. return Promise.resolve(setTimeout(this.onLoop.bind(this), this.duration * 1000) as return statement in onLoop() ? \$\endgroup\$ – Kleioz Feb 7 '18 at 10:08
  • \$\begingroup\$ Then I'm failing to understand the goal. Do you need to execute onLoop() a set amount of times? Do you need to do it indefinitely but everytime after some delay? It might also help what player is and what does play() do or return. \$\endgroup\$ – alrodseg Feb 7 '18 at 13:09
  • \$\begingroup\$ I need to call "this.player.setTime(5)" every 10 seconds for example, so that my video can loop indefinetly from 00:05 to 00:15. My implementatio nactually works (the parenthesis missing are a copy/paste mistake). Now what I was wondering is if there were a prettier way to implement it without passing the "resolve" as parameter. playeris a video player API (vimeo, but this is no big deal) play() launches the video and returns a Promise. So what I need to do is call onLoop() every 5 seconds until "this.isLooping" is set to false somewhere else in my app. \$\endgroup\$ – Kleioz Feb 7 '18 at 14:46
  • \$\begingroup\$ wOxxOm comment on my OP is, for example, a decent solution. Your suggestion is also correct but misses the setTimeout part. Which is causing me troubles to use with Promises :) \$\endgroup\$ – Kleioz Feb 7 '18 at 14:48
  • \$\begingroup\$ Please check the edited answer. I've just done a few changes to your code, but now it looks kinda redundant to have onStart() and onLoop() :D I hope it will give you an idea on how to proceed. If you need help tweaking it, just ask ;) \$\endgroup\$ – alrodseg Feb 7 '18 at 15:42

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.