node.js table search fails with promises in use

Question

I'm going to learn something here. I love to learn! Thanks in advance for any help!

Here's 30 lines of Node.js JavaScript. It's a standard table search. For simplicity, my "table" is a sequence of 16 integers, 0 to 15. Function compareRow reports whether a guessed row r has a row number below, equal, or above a row corresponding to a transaction t; t needs row number r = t. (Sure, the longer code is more complex.) I've removed everything I could to de-clutter for you.

Asynchronous issues (and some of you here in past questions) have encouraged me to use Promises. My longer code requires bluebird and gets identical results. This code uses JavaScript Promises.

'use strict'
function compareRow(r, t, low, high) {
  return new Promise(function(resolve, reject) {
    console.log('compareRow   r=' + r + '   t=' + t + '   low=' + low +
        '   high=', high)
    return resolve(r - t)
  })
}
function findRow(t) {
  let lo = 0, hi = 15, mid
  return new Promise(function(resolve, reject) {
    let mustBreak = false
    let count = 0, maxCount = 3
    do {
      mid = Math.floor((lo + hi) / 2)
      compareRow(mid, t, lo, hi)
      .then(function(ans, error) {
        console.log('findRow got compareRow(' + mid + ')=' + ans)
        if (error) { mustBreak = true; return reject(error) }
        else if (ans === 0) { mustBreak = true; return resolve(mid) }
        else if (ans < 0) lo = mid
        else if (ans > 0) hi = mid
      })
    } while (!mustBreak && ++count < maxCount)
  })
}
findRow(2)
.then(function(ans1, err1) {
  console.log('for findRow(2):   ans=' + ans1 + '   err=' + err1)
})

Self-critique:

• The use of variable mustBreak is icky. At each place I've used mustBreak=true, I'd rather both break and return resolve(n) (or whatever).
• I recognize that within the callback functions, JavaScript treats my promise returns (return resolve(1), for example) as returns from the callback functions, not from the promise functions at the top of the function. Hence, at each such call, JavaScript tracks two chains of execution: one for the resolve or reject and one for code listed after the callback. Ick. That second one is an unintended chain of execution.
• JSHint doesn't like the callback function involving ans. It correctly recognizes the function to be inside the for loop. Ick, except that this good advice in JSHint may not best account for callback functions.

Based on console.log messages I have since removed, execution runs as I wanted through line 17 (compareRow(mid, t, lo, hi)). I expected console.log would show me something like:

compareRow   r=7   t=2   low=0   high= 15
findRow got compareRow(7)=5
compareRow   r=3   t=2   low=0   high= 7
findRow got compareRow(3)=1
compareRow   r=1   t=2   low=0   high= 3
findRow got compareRow(1)=-1
compareRow   r=1   t=2   low=1   high= 3
findRow got compareRow(3)=1
compareRow   r=2   t=2   low=1   high= 3
findRow got compareRow(3)=0
for findRow(2):   ans=2   err=undefined

Instead, I got:

compareRow   r=7   t=2   low=0   high= 15
compareRow   r=7   t=2   low=0   high= 15
compareRow   r=7   t=2   low=0   high= 15
findRow got compareRow(7)=5
findRow got compareRow(7)=5
findRow got compareRow(7)=5

It appears

• Execution gets into compareRow (possibly in a timely way), but doesn't get on time into the callback function that receives that answer, resulting in an infinite loop.
• Or at least, that loop would be infinite except for the loop counter that limits the loop to three cycles.
• Given that the loop stops, it appears no accident that callback processes the answers (findRow got compareRow(7)=5) and gets called the same number of times the loop ran (three times).
• By the time that callback code runs, the code to process the output is no longer listening.

Questions:

• Is there a way I can encourage the JavaScript scheduler to process the code as I intend? (This seems an entirely standard use case for Promises!)

• How can I write this code better?

EDIT: Wording and question changes; all to the same effect as the original post. Deleted many lines of code. Corrected a syntax error.

Answer 1

@Bergi: Well done indeed! If you care about points in Stack Overflow, please post some wording in an answer and I'll make yours the accepted solution.

I said at the start I'd learn something here. I did, indeed. The following is an anti-pattern because JavaScript doesn't interrupt the loops "for the promises you create inside its body. You cannot use such loops in asynchronous code."

NOTE: Don't do this at home!
do {
  callPromiseFunction()
  .then(function(results, error) {
    // do something with results
  })
} while (/*some condition*/)

The alternative: "Use a recursive approach instead, wherein you can call the next step from an asynchronous promise callback."

I also learned that the following works. I don't know that it's to be emulated, but it proved sufficient on this code.

NOTE: This was sufficient in this code to start the promise chain.
return new Promise
.all([callNonrecursivePromiseFunc(1), callNonrecursivePromiseFunc(2)]
.then(function(result, error) {
  callRecursivePromiseFunction(result)
  .then(function(results, error) {
    // do something with results
  })
})

Here's how I used Bergi's advice to change my code. It works. Finally. Whew! Again, I've removed lots of error checking and such to reduce the bulk.

'use strict'
let Promise = require('bluebird')
function compareRow(t, r, low, high) {
  return new Promise(function(resolve, reject) {
    console.log('compareRow   t=' + t + '   r=' + r + '   low=' + low +
        '   high=', high)
    let recursion = (r === null)  // distraction; skip it
    if (recursion) {
      r = Math.floor((low + high) / 2)
      if (r < t) low = r
      else if (r > t) high = r
      else if (r === t) return resolve(r)
      compareRow(t, null, low, high)  // recursion; this is the point
      .then(function(result, error) {
        return resolve(result)
      })
    } else {
      return resolve(r - t)  // distraction; skip it
    }
  })
}
function findRow(t) {
  return new Promise(function(resolve, reject) {
    let lo = 0, hi = 15
    console.log('findRow   t=' + t + '   lo=' + lo + '   hi=' + hi)
    return new Promise
    .all([compareRow(t, lo, lo, hi), compareRow(t, hi, lo, hi)])
    .then(function(result, error) {
      let ans = result[0]
      console.log('findRow got compareRow(' + lo + ')=' + ans)
      if (ans === 0) return resolve(lo)
      ans = result[1]
      console.log('findRow got compareRow(' + hi + ')=' + ans)
      if (ans === 0) return resolve(hi)
      compareRow(t, null, lo, hi)  // this is a recursive call
      .then(function(result, error) {
        return resolve(result)
      })
    })
  })
}
findRow(2)
.then(function(ans1, err1) {
  console.log('for findRow(2):   ans=' + ans1 + '   err=' + err1)
})

Comment:

• Please note the use of bluebird Promises. This code fails with JavaScript Promises.
• With the recursive approach:
  • The need for variable mustBreak went away. That was an "Ick".
  • Each time I call "resolve" or "reject", I'm in the same scope as the declarations. That was an "Ick".
  • At no point do I define a function within a loop. JSHint is happier. That was an "Ick".
• This code has some unwelcome complexity in that I use compareRow for both a recursive and a non-recursive purpose. Ick. I'm falling short of elegance here.