3

Background

I am trying to create a factory function that executes a specific async function with a given delay.

For the purposes of this question, this will be the async function I refer to:

/*
 *  This is a simulation of an async function. Be imaginative! 
 */
let asyncMock = function(url) {
    return new Promise(fulfil => {

        setTimeout(() => {
            fulfil({
                url,
                data: "banana"
            });
        }, 10000);

    });
};

This function takes an url and it returns a JSON object containing that URL and some data.

All around my code, I have this function called in the following way:

asyncMock('http://www.bananas.pt')
.then(console.log);

asyncMock('http://www.berries.com')
.then(console.log);

//... badjillion more calls

asyncMock('http://www.oranges.es')
.then(console.log);

Problem

The problem here is that all these calls are made at exactly the same time, thus overloading the resources that asyncMoc is using.

Objective

To avoid the previous problem, I wish to delay the execution of all calls to asyncMoc by Xms.

Here is a graphic with what I pretend:

delayed_requests

To achieve this I wrote the following approaches:

  1. Using Promises
  2. Using setInterval

Using Promises

let asyncMock = function(url) {
  return new Promise(fulfil => {

    setTimeout(() => {
      fulfil({
        url,
        data: "banana"
      });
    }, 10000);

  });
};

let delayFactory = function(args) {

  let {
    delayMs
  } = args;

  let promise = Promise.resolve();

  let delayAsync = function(url) {
    return promise = promise.then(() => {

      return new Promise(fulfil => {
        setTimeout(() => {
          console.log(`made request to ${url}`);
          fulfil(asyncMock(url));
        }, delayMs);
      });
    });
  };

  return Object.freeze({
    delayAsync
  });
};

/*
 *  All calls to any of its functions will have a separation of X ms, and will
 *  all be executed in the order they were called. 
 */
let delayer = delayFactory({
  delayMs: 500
});

console.log('running');

delayer.delayAsync('http://www.bananas.pt')
  .then(console.log)
  .catch(console.error);

delayer.delayAsync('http://www.fruits.es')
  .then(console.log)
  .catch(console.error);

delayer.delayAsync('http://www.veggies.com')
  .then(console.log)
  .catch(console.error);

This factory has a function called delayAsync that will delay all calls to asyncMock by 500ms.However, it also forces the nest execution of the call to wait for the result of the previous one - which in not intended.

The objective here is to make three calls to asyncMock within 500ms each, and 10s after receive three responses with a difference of 500ms.

Using setInterval

In this approach, my objective is to have a factory which has an array of parameters. Then, every 500ms, the timer will run an executor which will take a parameter from that array and return a result with it:

/*
 *  This is a simulation of an async function. Be imaginative! 
 */
let asyncMock = function(url) {
  return new Promise(fulfil => {

    setTimeout(() => {
      fulfil({
        url,
        data: "banana"
      });
    }, 10000);

  });
};


let delayFactory = function(args) {

  let {
    throttleMs
  } = args;

  let argsList = [];
  let timer;

  /*
   *  Every time this function is called, I add the url argument to a list of 
   *  arguments. Then when the time comes, I take out the oldest argument and 
   *  I run the mockGet function with it, effectively making a queue.
   */
  let delayAsync = function(url) {
    argsList.push(url);

    return new Promise(fulfil => {

      if (timer === undefined) {

        console.log('created timer');
        timer = setInterval(() => {

          if (argsList.length === 0) {
            clearInterval(timer);
            timer = undefined;
          } else {
            let arg = argsList.shift();

            console.log('making  request ' + url);
            fulfil(asyncMock(arg));
          }
        }, throttleMs);

      } else {
        //what if the timer is already running? I need to somehow 
        //connect it to this call!
      }
    });
  };



  return Object.freeze({
    delayAsync
  });
};

/*
 *  All calls to any of its functions will have a separation of X ms, and will
 *  all be executed in the order they were called. 
 */
let delayer = delayFactory({
  delayMs: 500
});

console.log('running');

delayer.delayAsync('http://www.bananas.pt')
  .then(console.log)
  .catch(console.error);

delayer.delayAsync('http://www.fruits.es')
  .then(console.log)
  .catch(console.error);

delayer.delayAsync('http://www.veggies.com')
  .then(console.log)
  .catch(console.error);
// a ton of other calls in random places in code

This code is even worse. It executes asyncMoch 3 times without any delay whatsoever, always with the same parameter, and then because I don't know how to complete my else branch, it does nothing.

Questions:

  1. Which approach is better to achieve my objective and how can it be fixed?
6
  • 1
    I don't understand your objective, can you expand on it a bit? (Perhaps with reference to the questions I asked in this comment?) Commented Mar 7, 2017 at 15:08
  • Do you mean that url is always what was used in the last function call for all calls? If so, I don't have this problem with your code. Commented Mar 7, 2017 at 15:20
  • 1
    I improved the questions and added an image with what I pretend. I hope it is clear now because I don't see a way of explaining it better :S Commented Mar 7, 2017 at 15:29
  • You still haven't confirmed/rejected my item #2 from my previous comment: Does the promise from throttleAsync (I guess it's delayAsync now) need to resolve based on the resolution of asyncMock's promise? Commented Mar 7, 2017 at 15:31
  • I don't want the second call to wait for the response of the first one. This said if throttleAsync could resolve based on the request it took it would be awesome. I just don't see a way of doing it. Commented Mar 7, 2017 at 15:33

3 Answers 3

1

I'm going to assume you want the promises returned by delayAsync to resolve based on the promises from asyncMock.

If so, I would use the promise-based approach and modify it like this (see comments):

// Seed our "last call at" value
let lastCall = Date.now();
let delayAsync = function(url) {
  return new Promise(fulfil => {
    // Delay by at least `delayMs`, but more if necessary from the last call
    const now = Date.now();
    const thisDelay = Math.max(delayMs, lastCall - now + 1 + delayMs);
    lastCall = now + thisDelay;
    setTimeout(() => {
      // Fulfill our promise using the result of `asyncMock`'s promise
      fulfil(asyncMock(url));
    }, thisDelay);
  });
};

That ensures that each call to asyncMock is at least delayMs after the previous one (give or take a millisecond thanks to timer vagaries), and ensures the first one is delayed by at least delayMs.

Live example with some debugging info:

let lastActualCall = 0; // Debugging only
let asyncMock = function(url) {
  // Start debugging
  // Let's show how long since we were last called
  console.log(Date.now(), "asyncMock called", lastActualCall == 0 ? "(none)" : Date.now() - lastActualCall);
  lastActualCall = Date.now();
  // End debugging
  return new Promise(fulfil => {

    setTimeout(() => {
      console.log(Date.now(), "asyncMock fulfulling");
      fulfil({
        url,
        data: "banana"
      });
    }, 10000);

  });
};

let delayFactory = function(args) {

  let {
    delayMs
  } = args;

  // Seed our "last call at" value
  let lastCall = Date.now();
  let delayAsync = function(url) {
    // Our new promise
    return new Promise(fulfil => {
      // Delay by at least `delayMs`, but more if necessary from the last call
      const now = Date.now();
      const thisDelay = Math.max(delayMs, lastCall - now + 1 + delayMs);
      lastCall = now + thisDelay;
      console.log(Date.now(), "scheduling w/delay =", thisDelay);
      setTimeout(() => {
        // Fulfill our promise using the result of `asyncMock`'s promise
        fulfil(asyncMock(url));
      }, thisDelay);
    });
  };

  return Object.freeze({
    delayAsync
  });
};

/*
 *  All calls to any of its functions will have a separation of X ms, and will
 *  all be executed in the order they were called. 
 */
let delayer = delayFactory({
  delayMs: 500
});

console.log('running');

delayer.delayAsync('http://www.bananas.pt')
  .then(console.log)
  .catch(console.error);

delayer.delayAsync('http://www.fruits.es')
  .then(console.log)
  .catch(console.error);

// Let's hold off for 100ms to ensure we get the spacing right
setTimeout(() => {
  delayer.delayAsync('http://www.veggies.com')
    .then(console.log)
    .catch(console.error);
}, 100);
.as-console-wrapper {
  max-height: 100% !important;
}

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

6 Comments

The mathematical approach. My first tentative which failed blatantly. Can you explain a little bit about the math that you are doing? Thanks for the answer! (exactly what I was looking for!)
@Flame_Phoenix: Since we want to delay delayMs from the last scheduled call time (which can be in the future) or from now (whichever is later), lastCall - now + 1 tells us how far in the future (positive) or past (negative) lastCall is, in milliseconds. Then we add delayMs to that. If lastCall was in the past, that number will be smaller than delayMs; but if lastCall is in the future, it may be greater than delayMs. So we take the greater of the two values and use that as our delay. Then we remember when we scheduled that for, for next time. (The +1 is probably overkill.)
Hi T.J. Crowder, I believe that you can improve this code by removing the expression 'Math.max(delayMs, lastCall - now + 1 + delayMs)' as 'delayMs' < 'lastCall - now + delayMs' .
@SoldeplataSaketos: No, the first call is likely to have delayMs > lastCall - now + 1 + delayMs, and that can also be true if there's a long enough gap between calls.
Oh, true, I wasn't considering big gaps!! I suppose that the '+1' is mathematically there for some reason I don't get right now, but as the nature of JavaScript is not so exact in time, I suppose it's not needed
|
1

Okay, so here's my solution to your problem. Sorry I had to rewrite your code to better be able to understand it. I hope you can interpret it anyway and get something out of it.

Calls 500ms between eachother using Promises (JSFiddle):

function asyncFunc(url) {
    return new Promise(resolve => {
    setTimeout(function() {
        resolve({ url: url, data: 'banana' });
    }, 2000);
  });
}

function delayFactory(delayMs) {
  var delayMs = delayMs;
  var queuedCalls = [];
  var executing = false;

  this.queueCall = function(url) {
    var promise = new Promise(function(resolve) {
        queuedCalls.push({ url: url, resolve: resolve });
        executeCalls();
    });
    return promise;
  }

  var executeCalls = function() {
    if(!executing) {
      executing = true;
      function execute(call) {
        asyncFunc(call.url).then(function(result) {
            call.resolve(result);
        });
        setTimeout(function() {
            queuedCalls.splice(queuedCalls.indexOf(call), 1);
          if(queuedCalls.length > 0) {
            execute(queuedCalls[0]);
          } else {
            executing = false;
          }
        }, delayMs)
      }
      if(queuedCalls.length > 0) {
        execute(queuedCalls[0]);
      }
    }
  }
}

var factory = new delayFactory(500);
factory.queueCall('http://test1').then(console.log); //2 sec log {url: "http://test1", data: "banana"}
factory.queueCall('http://test2').then(console.log); //2.5 sec log {url: "http://test2", data: "banana"}
factory.queueCall('http://test3').then(console.log); //3 sec log {url: "http://test3", data: "banana"}
factory.queueCall('http://test4').then(console.log); //3.5 sec log {url: "http://test4", data: "banana"}

9 Comments

I don;t know who downvoted this, but I am upvoting it. Even if not correct, it shows effort. Yes, I can execute each function one after the other like that, but if I do it your way there won't be any delay bewteen the calls, and the second call will be forced to wait for the first one's result and so on, which is not pretended.
Ok, now i see. There will be 10 seconds delay between calls though as specified by the setTimeout. But I get your point of not wanting to wait on previous calls.
Actually, commenting downvotes is almost always not useful. Commenting what's wrong with an answer can be, though. In this case: This doesn't answer the question(s) at all. It doesn't account for the expressed desire for a delay between calls, it doesn't address promises vs. timers.
True, this was an attempt to answer the question in a simpler manner since i thought op was over complicating things.
Edited to a hopefully better answer.
|
1

Introduction

After reading both solutions, I have to say I am very thankful to both people who took their time to help me. It is moments like this (although rare) that make me proud of having a StackOverflow account.

This said, after reading both proposals, I came with one of my own, and I will explain which one I think is best and why.

My solution

My solution is based on @Arg0n's proposal, and it is a simplification/re-implementation of his code using the factory pattern in JavaScript and defended by Douglas Crockford, using ECMA6 features:

let asyncFunc = function(url) {
  return new Promise((resolve, reject) => {
    setTimeout(function() {
      resolve({
        url: url,
        data: 'banana'
      });
    }, 5000);
  });
};

let delayFactory = function(args) {
  let {
    delayMs
  } = args;
  let queuedCalls = [];
  let executing = false;

  let queueCall = function(url) {
    return new Promise((resolve, reject) => {

      queuedCalls.push({
        url,
        resolve,
        reject
      });

      if (executing === false) {
        executing = true;
        nextCall();
      }
    });
  };

  let execute = function(call) {

    console.log(`sending request ${call.url}`);

    asyncFunc(call.url)
      .then(call.resolve)
      .catch(call.reject);

    setTimeout(nextCall, delayMs);
  };

  let nextCall = function() {
    if (queuedCalls.length > 0)
      execute(queuedCalls.shift());
    else
      executing = false;
  };

  return Object.freeze({
    queueCall
  });
};

let myFactory = delayFactory({
  delayMs: 1000
});

myFactory.queueCall('http://test1')
  .then(console.log)
  .catch(console.log);

myFactory.queueCall('http://test2')
  .then(console.log)
  .catch(console.log);

myFactory.queueCall('http://test3')
  .then(console.log)
  .catch(console.log);

Why am I posting this extra solution? Because I think it is a vast improvement over Arg0n's proposal, for the following reasons:

  • No falsiness. Falsy values and expressions (like !executing) are a problem in JavaScript. I strongly recommend Appendix A: Awful parts of JavaScript.
  • Implements catch should the asyncMock fail
  • Use of Array.prototype.shift instead of Array.prototype.splice which is easier to read and improves performance.
  • No use of new keyword, no messing of the this reference
  • No inner functions. ESlint will thank you :P
  • Use of factories Douglas Crockford style

If you liked Arg0n's solution, I recommend you have a look at mine.

@Arg0n VS @T.J. Crowder ... FIGHT!

Which solution is better and why?

At first i was inclined to Arg0n's solution, because it took inspiration from one of my failed attempts and made it work. On itself, that is remarkable.

Furthermore, Timers in JavaScript have precision issues, and JavaScript also has issues when making computations with numbers (check 0.1 + 0.2 !== 0.3).

However, both solution use Timers. In fact, you need timers to achieve this behavior. Furthermore @T.J. Crowder's solution does not do arithmetic with floating points, but whole numbers, so his calculations are safe and sound.

One could point out that the Math library was a mistake in JavaScript imported from java, but honestly that is going to far and there is nothing wrong with it.

Furthermore, because T.J.'s solution does not have a data structure like Arg0n's solution has, its code is smaller as it encompasses less logic to maintain. There is no question from a technical point of view, his solution is the one to go for, in this specific case.

However, for those of you who don't master the math behind, Arg0n's avenue is a pretty solid one.

Conclusion

From a technical point of view, T.J.'s solution wins. However I can say that I enjoyed Arg0n's solution a lot, and specially my version of his post, which is the one I am likely to use.

I hope this post helps someone in the future !

1 Comment

Great work! Indeed my code is not perfect, tho i wrote it in like 20 minutes just to get you in the right direction :)

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.