0

I have a complicated javascript function with promises inside it.

Here is my code : 

var chunkProjectList = function(list, project, accounts) {
  return new Promise((resolve, reject) => {
    // init delta
    let delta = 5
    if ((accounts.length * delta) > list.length ) {
      delta = (list.length / accounts.length)
    }
    // init chunked list
    let chunkedList = []
    for (let i = 0; i < accounts.length; i++) {
      chunkedList.push([])
    }
    // loop on all users
    for (let i = 0; i < list.length; i++) {
      isInBlacklist(list[i], project.id)
      .then(() => { // current user is in the blacklist
        ProjectModel.deleteElementFromList(project.id, list[i])
        if (i === list.length - 1) {
          // no screen_names available, cu lan
          return resolve(chunkedList) 
        }
      })
      .catch(() => { // we continue
        let promises = []
        for (let j = 0; j < accounts.length; j++) {
          promises[j] = FollowedUserModel.getFollowedUserByScreenName(list[i], accounts[j].id)
        }
        // we checked a screen_name for all accounts
        Promise.all(promises)
        .then((rows) => {
          for (let k = 0; k < rows.length; k++) {
            if (rows[k][0].length === 0 && chunkedList[k].length < delta && list[i] !== '') {
              console.log('we push')
              chunkedList[k].push(list[i])
              break
            }
            console.log('we cut (already followed or filled chunklist)')
            if (k === rows.length - 1) {

              // determine if is cancer screen_name or just filled chunklists
              for (let l = 0; l < chunkedList.length; l++) {
                if (chunkedList[l].length < delta) {
                  console.log('we cut because nobody wants ya')   
                  ProjectModel.deleteElementFromList(project.id, list[i])
                  ProjectModel.addElementToBlackList(project.id, list[i])
                }
                if (l === chunkedList.length - 1) {
                  // you are all filled, cu lan
                  return resolve(chunkedList)
                  break
                }
              }
            }
          }
          if (i === list.length - 1) {
          // no screen_names available, cu lan
            over = 1
            return resolve(chunkedList)
          }
        })
      })
    }
  })
}

My program is looping on a list of usernames, and tries to share it between the accounts, with a limit called 'delta' for each account

example: My list contains 1000 usernames, the delta is 100 and I have 4 accounts The expected result is an array of arrays, containing for each arrays, 100 usernames.

chunkedList (array) => ( array 1: length 100, array 2: length 100 ... )

The problem I have is the following :

When the delta is reached for all the chunkedList arrays, I just wanna exit this function, and stop every work inside, even the running promises. Just when I 'return resolve(chunkedList)'.

But the program continues to run, and it's a real problem for performances.

I know it's difficult to understand the code. Thanks

Adam Muller
  • 7
  • 5
  • I will admit I have only briefly skimmed the code. Based on your description, maybe you could add `let abortFlag = false` to the very top of the function, assign it to `true` when the delta is reached, and add `if` checks for it in the promise resolver functions so that they resolve early if they see it? – Katana314 Sep 08 '16 at 13:24
  • I suggest you spend 30 min. refactoring into separate functions each returning a promise. Then it's easier for us to comment on it. It's a bit timeconsuming to read the code as is – Michael Frost Billing Sep 08 '16 at 13:55
  • Start by avoiding the [Promise constructor antipattern](http://stackoverflow.com/q/23803743/1048572)! – Bergi Sep 08 '16 at 14:08

1 Answers1

0

I am having difficulty understanding what your code is trying to accomplish. However, it looks like you could split your code up into a few separate promise functions. This would help clarity and should allow you stop execution. Without being able to test your code, the main problem seems to be caused by your main loop. In your code you have the following:

.then(() => {
    ....
    return resolve(chunkedList);
}

The "return" statement here is only going to return from the inner .then() call and not from your main Promise. Thus, your main loop will continue to execute all of its code on the whole array. Essentially, it seems that your loop will continue to modify your main Promise's resolved value and never return until the whole loop has completed.

My recommendation is that you split out your main loop contents to be a separate method that takes a list as a parameter. This method will return a Promise. With that method, you can create a Promise.all() using your main loop and then add a .catch() method. The catch() method will occur when you reject one of the method calls (therefore not completing the rest of the promises in the rest of the Promise.all array).

I apologize if my above suggestion does not make any sense. I am trying to write this quickly. In summary, if you can split out the different steps of your method into their own methods that return their own promises, you can use promises very effectively allowing you to chain, execute multiple tasks in parallel, and/or execute tasks sequentially.

Ryan Mitchener
  • 16
  • 2