0
let posts = [];
for(var i = user.posts.length - 1; i >= 0; i--){
  posts.push(user.posts[i]);
}
for(var i = 0; i < user.friends.length; i++){
 let query = {username:user.friends[i]};
 User.findOne(query, function(err, theuser){
  if(err) throw err;

  if(theuser){
    if(theuser.posts.length > 0){
      for(var j = theuser.posts.length - 1; j >= 0; j--){
        posts.push(theuser.posts[j]);
      }
    }
  }
});
} 
return posts;

So my problem is that when I call this function, posts is not returning correctly. When i run this i want to return an array of posts and the first for loop works good but when i want to push an element to the array from the nest for loop where the var = j, it is not updating the posts array. I think it might have something to do with async function but im not sure on how to implement it in my case.

Ethan Brimhall
  • 357
  • 3
  • 10
  • I don't see anything obviously wrong with your code. It may help to see how you're testing the code and what values you're seeing returned vs what you're expecting. – schu34 Jan 19 '19 at 14:26
  • 2
    Why is this tagged `async`/`await`? Do plan using promises? – Bergi Jan 19 '19 at 14:29
  • 2
    `.findOne()` is asynchronous. `return posts` is executed before `posts.push(...)`. – Andreas Jan 19 '19 at 14:33

1 Answers1

1

You cannot return from asynchronous functions; at least not like you're doing.

let posts = []
// ...
for(var i = 0; i < user.friends.length; i++){
  // ...
  User.findOne(query, function(err, theuser){
    // nothing here happens before `return posts` is called below
  })
} 
return posts;

This should help put you back on the right path -

// reverse copy of user posts
const posts =
  [ ...user.posts ].reverse()

// "promisified" findOne
const findOne = query =>
  User.findOne
    ( query
    , (err, res) =>
        err
          ? Promise.reject(err)
          : Promise.resolve(res)
    )

Promise
  .all(user.friends.map(username => findOne({ username }))
  .map(friend => friend.posts.reverse())
  .then(friendPosts => posts.concat(friendPosts)
  .then
     ( allPosts =>
         // do something with all posts here
     )

The alternative is async/await. Here we also use util.promisify instead of rewriting findOne by hand -

const { promisify } =
  require('util')

const findOne =
  promisify(User.findOne)

// below, `async` functions always return a promise
const allPostsAndFriendsPosts = async (user) =>
{ // write normal synchronous code here
  let res =
    [ ...user.posts ].reverse()

  // inside async functions, we can `await` a promise
  const friends =
    await Promise.all(user.friends.map(username => findOne({ username }))

  // then use the promised value synchronously
  for (const f of friends)
    res = res.concat(f.posts.reverse())

  return res // <-- returns a promise
}

allPostsAndFriendsPosts(user)
  .then
    ( allPosts =>
        // do something with all posts
    )

async and await are the tandem duo of this era. I just want to offer an appreciation of their combined capability. Say you have a database -

const DB = 
  { '/0': { a: 'a', _link: '/1' }
  , '/1': { b: 'b', _link: '/2' }
  , '/2': { c: 'c', d: 'd', _link: '/3' }
  , '/3': { e: 'e' }

  , '/4': { f: 'f', _link: '/5' }
  , '/5': { g: 'g' }

  // ...
  }

Where each node has a path like /0, /1, etc, and nodes can link to other nodes using the _link property. Chains of link->link->link are not limited in length. Given a starting node, the goal is to produce the entire sequence of nodes –

recursiveGet ('/0') .then (console.log, console.error)
// [ { a: 'a' }, { b: 'b' }, { c: 'c', d: 'd' }, { e: 'e' } ]

recursiveGet ('/4') .then (console.log, console.error)
// [ { f: 'f' }, { g: 'g' } ]

recursiveGet ('/99') .then (console.log, console.error)
// Error: path not found: /99

We need some way to define a loop, some way to initialize it, some way to do the next loop, and finally some way to say when the loop is done. Oh and everything has to be async.

It's a tall order, but async and await are up to the task. When writing generic functions, we keep things as generic as possible to maximize reusability -

const asyncUnfold = async (loop, init) =>

  // call the user's loop with
  loop

      // the "next" function
      // accepts two arguments
      // 1. the item to add to the result
      // 2. the next accumulator
    ( async (x, acc) =>
        // the item is prepended to the recursive result
        [ x, ...await asyncUnfold (f, acc) ]

      // the "done" function
      // accepts one argument
      // 1. then final item of the result
    , async (x) => [ x ]

      // initial accumulator
    , init
    )

Given a get function that does not do a recursive query -

const get = async (url = '') =>
  fetch (url) .then (res => res .json ())

We can now write recursiveGet using asyncUnfold -

const recursiveGet = async (initUrl) =>

  // use our new magic wand
  asyncUnfold

      // our loop
      // receives 3 arguments
      // 1. the "next" function
      // 2. the "done" function
      // 3. the accumulator
    ( async (next, done, { _link, ...res }) =>

        // if we have a _link ...
        _link

          // add res to the output
          // the next step is get(_link)
          ? next (res, await get (_link))

          // otherwise there is no _link
          // call done with the last result
          : done (res)

    // initial accumulator
    , await get (initUrl)
    )

And all without having to touch Promise, reject, resolve, or then. I hope this gives you a glimpse of the capable expressions that can be made using async and await. Verify the results in your browser below -

const asyncUnfold = async (f, init) =>
  f ( async (x, acc) => [ x, ...await asyncUnfold (f, acc) ]
    , async (x) => [ x ]
    , init
    )
    
const get = async (url = '') =>
  fetch (url) .then (res => res .json ())
  
const recursiveGet = async (initUrl) =>
  asyncUnfold
    ( async (next, done, { _link, ...res }) =>
        _link
          ? next (res, await get (_link))
          : done (res)
    , await get (initUrl)
    )
 
const DB = 
  { '/0': { a: 'a', _link: '/1' }
  , '/1': { b: 'b', _link: '/2' }
  , '/2': { c: 'c', d: 'd', _link: '/3' }
  , '/3': { e: 'e' }
  , '/4': { f: 'f', _link: '/5' }
  , '/5': { g: 'g' }
  }

// fake fetch for demo
const fetch = (url = '') =>
  DB[url] === undefined
    ? Promise .reject (Error(`path not found: ${url}`)) .then (delay)
    : Promise .resolve ({ json: () => DB[url] }) .then (delay)

// fake delay for demo
const delay = (x, ms = 250) =>
  new Promise (r => setTimeout (r, ms, x))
  
recursiveGet ('/0') .then (console.log, console.error)
// [ { a: 'a' }, { b: 'b' }, { c: 'c', d: 'd' }, { e: 'e' } ]
Mulan
  • 129,518
  • 31
  • 228
  • 259