0

Ultimately my knowledge is so bad as I started just a few hours ago. But in my head this should work:

getAll: function(callback){
    User.find({}, function (err, data) {
        if (err) return console.error(err);
        return data;
    });
}

var users = api.getAll(); 
console.log(users); //undefined

But for some reason I have to pass it as a callBack to get the data working like so:

getAll: function(callback){
    User.find({}, function (err, kittens) {
        if (err) return console.error(err);
        callback(kittens);
    });
}
var users = api.getAll(function (data) {
    console.info(data); //does output data
});

How can I get option one, the easier to read of the 2 to work?

Jamie Hutber
  • 26,790
  • 46
  • 179
  • 291
  • Mongoose's `find` is asynchronous, as is anything that requires a callback. So at the time of assignment `api.getAll()` isn't anything relevant. And readability is rather subjective. To me, it'd be more readable to properly use promises, or even well-written callbacks. – Tony Jun 19 '15 at 23:46
  • This would be a good place to start getting your head around dealing with async: http://stackoverflow.com/questions/23667086/why-is-my-variable-unaltered-after-i-modify-it-inside-of-a-function-asynchron – JohnnyHK Jun 20 '15 at 00:07

1 Answers1

1

Unfortunately you can't. When you execute a function, you get whatever you return from it. If you return nothing you get undefined by default.

Instead of having your readability based on getting the user, you should have it based on what you're going to do with it once you've gotten it. Presumably you'll have some action(s) that you'll want to perform on the user, right? This is where Promises can make your code look significantly better than callbacks.

Here's that code re-written with Promise

getAll: function(callback){
    return User.findAsync({});
}
api.getAll()
.then(console.log)
.catch(console.error); 

Since all you wanted to do was "perform" console.log on your users (and/or console.error on err in case that happens) that's exactly what the above code does.

Note that I used findAsync, which came from Bluebird's promisification

But more realistically you'd have functions that do other things to/with your user

api.getAll()
.then(filterSome)
.then(manipulateSome)
.then(sendSomewhere)
.catch(console.error); 

Each of those functions might look something like this:

function filterSome(users){
    return users.reduce(function(p,e){
        if(…) p.push(e);
        return p;
},[]);  }

They can even be async functions in which case you'll have to use more Promises:

function manipulateSome(users){
    return new Promise.all(user.map(function(user){
        return new Promise(function(resolve){
            someAsyncFunction(user, function(err, result){
                user.someProp = result.manipulated;
                resolve(user);
}); }); }));}

If that still looks messy, then know that with upcoming ES7 all the above would get a syntactic sugar and you would do something like this:

getAll: async function(callback){
    return User.findAsync({});
}
try{
    var users = await api.getAll();
    console.log(users);
catch(e){
    console.error(e)
}

You could start using it today with transpilers like BabelJS

laggingreflex
  • 32,948
  • 35
  • 141
  • 196
  • What a great answer. Thank you, I haven't heard of the old promise what not in node, not enough to ever consider using it and bluebird certainly makes it a lot easier after reading up about them. Nice one – Jamie Hutber Jun 20 '15 at 10:29