-1

I constantly run into problems with this pattern with callbacks inside loops:

while(input.notEnd()) {
    input.next();
    checkInput(input, (success) => {
        if (success) {
            console.log(`Input ${input.combo} works!`);
        }
    });
}

The goal here is to check every possible value of input, and display the ones that pass an asynchronous test after confirmed. Assume the checkInput function performs this test, returning a boolean pass/fail, and is part of an external library and can't be modified.

Let's say input cycles through all combinations of a multi-code electronic jewelry safe, with .next incrementing the combination, .combo reading out the current combination, and checkInput asynchronously checking if the combination is correct. The correct combinations are 05-30-96, 18-22-09, 59-62-53, 68-82-01 are 85-55-85. What you'd expect to see as output is something like this:

Input 05-30-96 works!
Input 18-22-09 works!
Input 59-62-53 works!
Input 68-82-01 works!
Input 85-55-85 works!

Instead, because by the time the callback is called, input has already advanced an indeterminate amount of times, and the loop has likely already terminated, you're likely to see something like the following:

Input 99-99-99 works!
Input 99-99-99 works!
Input 99-99-99 works!
Input 99-99-99 works!
Input 99-99-99 works!

If the loop has terminated, at least it will be obvious something is wrong. If the checkInput function is particularly fast, or the loop particularly slow, you might get random outputs depending on where input happens to be at the moment the callback checks it.

This is a ridiculously difficult bug to track down if you find your output is completely random, and the hint for me tends to be that you always get the expected number of outputs, they're just wrong.

This is usually when I make up some convoluted solution to try to preserve or pass along the inputs, which works if there is a small number of them, but really doesn't when you have billions of inputs, of which a very small number are successful (hint, hint, combination locks are actually a great example here).

Is there a general purpose solution here, to pass the values into the callback as they were when the function with the callback first evaluated them?

TheEnvironmentalist
  • 2,694
  • 2
  • 19
  • 46
  • If you want objects to not be affected by operations made on them elsewhere, then you have to make a copy of the appropriate properties and pass the copy. Objects are passed by pointer so any changes someone else makes to the object while you have the pointer will affect you. Nothing you can do about that directly. So, if you don't want to be affected, then either pass a copy or when you first receive the object, make a copy of the relevant properties or of the whole object. – jfriend00 Mar 06 '17 at 02:41
  • @jfriend00 It makes no difference. If you pass the input as a copy, then you need to store that copy somewhere while the function runs so it can be accessed by the callback, and as I wrote above storing every possible value is often highly impractical – TheEnvironmentalist Mar 06 '17 at 02:45
  • this question is confusing as to what is actually going wrong. I suggest you create a snippet or link an example of what your code is currently doing with the unexpected results – A. L Mar 06 '17 at 02:55
  • Then, I don't understand what you are asking. You either want to access the live object with whatever its current value is or you want to access a saved copy that isn't being changed by other code. Which is it? – jfriend00 Mar 06 '17 at 02:55
  • It may be the problem here is that you've embeded an asynchronous operation in the middle of a `while` loop and that's causing your problem. The while loop will run to completion while the async loop has its own timing. This is probably just the wrong type of code when trying to loop using async operations. – jfriend00 Mar 06 '17 at 02:58
  • But, your question does not explain the actual goal of the code or what the `input` object is so we can't exactly know what to recommend. You've described a problem without describing what you're trying to accomplish. The code is wrong so it doesn't help describe the desired goal. Please back up and describe what you are trying to accomplish (not just your problem). – jfriend00 Mar 06 '17 at 02:58
  • @jfriend00 Done, let me know if you need more information – TheEnvironmentalist Mar 06 '17 at 04:36
  • As my answer shows, you can either sequence your async operations so `input` is not changed before your async operation is done and thus it will have the desired value when you want to use it OR you can make a copy in a private scope of desired properties of the object so your copy can't be changed even though the core object is modified by other operations. Those are basically your two choices. My answer covers several ways to implement these. – jfriend00 Mar 06 '17 at 04:40
  • @jfriend00 If the method of incrementing `input` is fairly predictable, but storing all of the values isn't practical, how feasible would it be to reconstruct `input` and then associate each value to the appropriate callback call? – TheEnvironmentalist Mar 06 '17 at 04:48
  • @TheEnvironmentalist - You'd have to show all the code for the `input` object. As it stands now, we know NOTHING about what happens inside the object or what part of the object state you are trying to preserve. So, we're blind here and have no idea what is going on inside the object. Therefore, we don't have enough info to offer any idea on that new question. – jfriend00 Mar 06 '17 at 04:52
  • @jfriend00 I can give you a sample model, though this is a pattern I run into fairly often so beyond basic structure it's not very helpful to the problem in general. Let's say I'm trying to brute force a 2048-bit password system. Clearly, the full list of possible passwords, each an `input`, isn't feasible to store, but the process for checking passwords, `checkInput`, is just a request to a network service, so computationally the process is cheap, but it takes a while per check because of time spent waiting on the network. So checks in sequence are very inefficient, and storage is impossible. – TheEnvironmentalist Mar 06 '17 at 05:15
  • In the password check example, you would have to save the password that you were checking so if it passes, you know what it is. That's all you'd have to save and you only have to save it for the duration of the async operation, not any longer than that. I guess I don't understand what sort of magic bullet you're looking for. For a more complicated object, you could try to save `diffs` from one state to the next and be able to use that to reconstruct a prior state, but that is not a generic solution, that is very specific to what the object is and how it works and what mutable state it has. – jfriend00 Mar 06 '17 at 05:20
  • Also, in the password check example, you're not going to have tens of thousands of simultaneous async checks in flight at the same time. That's just not practical for the receiving server. So you'll have a limited number of checks all in flight at the same time (maybe dozens or at most a hundred) and that puts a limit on how much state you need to save at once. As soon as the async operation is done, the state you saved will be GCed. – jfriend00 Mar 06 '17 at 05:22
  • It drives us a bit nuts here when people try to ask generic questions without disclosing the relevant code because it's like trying our hands behind our backs or asking us to code with blinders on. Answers can always be made more useful if we can see all the relevant code. It's much easier to generalize an answer from a specific problem and specific code than it is to try to generalize an answer from a vague word description that doesn't include important details. I'm trying to help as much as I can, but you have my hands tied behind my back right now as you withhold the real code. – jfriend00 Mar 06 '17 at 05:25
  • @jfriend00 The actual situation is relatively similar. With permission from a weather service, I'm trying to predict about 500 bytes worth of historical weather data using ridiculous amounts of modeled sensor data. The sensor data is terabytes and terabytes in size, but can be reconstructed losslessly using the model I'm working on. The `input.combo` is the current prediction to test, `input.next` generates the next prediction, and `checkInput` checks the prediction with the service against an accuracy threshold. The data is binary, but structurally entirely predictable, though hard to explain – TheEnvironmentalist Mar 06 '17 at 05:41
  • If any iteration of the data can be reconstructed losslessly, then it seems you just need to write an algorithm to do that upon demand so you don't have to save every state. Nothing we can help with since we don't have any of the info to participate (that's what makes this frustrating for us to try to help without any actual code). – jfriend00 Mar 06 '17 at 05:48
  • @jfriend00 sorry about that, doing everything I can :) – TheEnvironmentalist Mar 06 '17 at 06:07

2 Answers2

2

If you want to iterate one async operation at a time, you cannot use a while loop. Asynchronous operations in Javascript are NOT blocking. So, what your while loop does is run through the entire loop calling checkInput() on every value and then, at some future time, each of the callbacks get called. They may not even get called in the desired order.

So, you have two options here depending upon how you want it to work.

First, you could use a different kind of loop that only advances to the next iteration of the loop when the async operation completes.

Or, second, you could run them all in a parallel like you were doing and capture the state of your object uniquely for each callback.

I'm assuming that what you probably want to do is to sequence your async operations (first option).

Sequencing async operations

Here's how you could do that (works in either ES5 or ES6):

function next() {
    if (input.notEnd()) {
        input.next();
        checkInput(input, success => {
            if (success) {
                // because we are still on the current iteration of the loop
                // the value of input is still valid
                console.log(`Input ${input.combo} works!`);
            }
            // do next iteration
            next();
        });
    }
}
next();

Run in parallel, save relevant properties in local scope in ES6

If you wanted to run them all in parallel like your original code was doing, but still be able to reference the right input.combo property in the callback, then you'd have to save that property in a closure (2nd option above) which let makes fairly easy because it is separately block scoped for each iteration of your while loop and thus retains its value for when the callback runs and is not overwritten by other iterations of the loop (requires ES6 support for let):

while(input.notEnd()) {
    input.next();
    // let makes a block scoped variable that will be separate for each
    // iteration of the loop
    let combo = input.combo;
    checkInput(input, (success) => {
        if (success) {
            console.log(`Input ${combo} works!`);
        }
    });
}

Run in parallel, save relevant properties in local scope in ES5

In ES5, you could introduce a function scope to solve the same problem that let does in ES6 (make a new scope for each iteration of the loop):

while(input.notEnd()) {
    input.next();
    // create function scope to save value separately for each
    // iteration of the loop
    (function() {
        var combo = input.combo;
        checkInput(input, (success) => {
            if (success) {
                console.log(`Input ${combo} works!`);
            }
        });
    })();
}
jfriend00
  • 683,504
  • 96
  • 985
  • 979
  • In my particular case `input` is a large object, and there are billions of values that are not practical to store simultaneously, so option 2 of storing everything is out. The `checkInput` function is computation cheap but time expensive, so option 1 doesn't work either. I wouldn't have asked the question if it was easy :) – TheEnvironmentalist Mar 06 '17 at 04:40
  • @TheEnvironmentalist - You either sequence the operations OR you copy the needed properties. There are no other choices. Pick which one works better for you. You seem to be looking for a magic bullet that does not exist. You seem to be asking for a way to mutate an object by running lots of other operations on it in parallel, but also keep the ability to access prior versions of the object without making a copy. That's just not possible (in any language that I know of). – jfriend00 Mar 06 '17 at 04:49
  • @TheEnvironmentalist - Did this answer your question? – jfriend00 Mar 24 '17 at 04:19
  • Quite well I might add – TheEnvironmentalist Mar 24 '17 at 05:24
0

You could use the new feature async await for asynchronous calls, this would let you wait for the checkInput method to finish when inside the loop.

You can read more about async await here

I believe the snippet below achieves what you are after, I created a MockInput function that should mock the behaviour of your input. Note the Async and await keywords in the doAsyncThing method and keep an eye on the console when running it.

Hope this clarifies things.

function MockInput() {
  this.currentIndex = 0;
  this.list = ["05-30-96", "18-22-09", "59-62-53", "68-82-0", "85-55-85"];
  
  this.notEnd = function(){
    return this.currentIndex <= 4;
  };
  
  this.next = function(){
      this.currentIndex++;
  };
  
  this.combo = function(){
      return this.list[this.currentIndex];
  }
}

function checkInput(input){
  return new Promise(resolve => {
      setTimeout(()=> {
        var isValid = input.currentIndex % 2 > 0; // 'random' true or false
        resolve( `Input ${input.currentIndex} - ${input.combo()} ${isValid ? 'works!' : 'did not work'}`);
      }, 1000);
  });
}

async function doAsyncThing(){
  var input = new MockInput();
  
  while(input.notEnd()) {
      var result = await checkInput(input);
      console.log(result);
      input.next();
  }
  
  console.log('Done!');
}

doAsyncThing();
Daniel Ormeño
  • 2,743
  • 2
  • 25
  • 30
  • It's pretty bizarre to wrap `checkInput()` in a promise and still use a callback for the results. That's generally not considered a good design practice. The result should be the resolved value of the promise. – jfriend00 Mar 06 '17 at 03:24