search

max-mapper / art-of-node

:snowflake: a short introduction to node.js
https://github.com/maxogden/art-of-node#the-art-of-node
Other
9.81k stars 854 forks source link

Can we simplify the callbacks pseudocode please? #40

Closed samjewell closed 8 years ago

samjewell commented 9 years ago

I feel like this is much more complicated than the concrete callbacks example we've just encountered.

function addOne(thenRunThisFunction) {
  waitAMinute(function waitedAMinute() {
    thenRunThisFunction()
  })
}

addOne(function thisGetsRunAfterAddOneFinishes() {})

Shouldn't thenRunThisFunction() be called one line further down, as shown below, for true reflection of the concrete example before this part?

function addOne(thenRunThisFunction) {
  waitAMinute(function waitedAMinute() {
  })    
  thenRunThisFunction()
}

addOne(function thisGetsRunAfterAddOneFinishes() {})

But then, is waitedAMinute even invoked at any point at all? This would make much more sense to me:

function addOne(thenRunThisFunction) {
  waitAMinute()
  thenRunThisFunction()
}

addOne(function thisGetsRunAfterAddOneFinishes() {})

Of if the point is that waitAMinute() is async, while waitedAMinute is either sync, or a cb, then can we name them that way? eg. waitAMnuteAsync() and waitedAMinuteCallback()

samjewell commented 9 years ago

ps. I can submit a PR, just let me know which of the above things you'd prefer. Upon reflection probably the very last suggestion right?

zeke commented 8 years ago

@samjewell I'm not sure your example will work, as the thenRunThisFunction will be invoked before the asynchronous waitAMinute callback is called.

If you are still stuck on this (which I doubt as it's been over a year :dizzy_face:), please open a PR. Thanks!

samjewell commented 8 years ago

what would you say to renaming waitAMinute to waitAMinuteAsync? For extra clarity...

zeke commented 8 years ago

The fact that one of waitAMinute's arguments is a function is a clear indication that it's an asynchronous function. But if you think changing the name makes it more clear, please send a PR and I'll have a look.

ferusinfo commented 8 years ago

I think that for us, developers - this is clear that this function will be an async one. But if we want to clarify everything related with javascript callback functions, their (people who are learning from this repo) brains are already melting with that amount of information. We need to treat them (without insulting!) like kids who are learning alphabet. Everything just must be big and bold ;)

samjewell commented 8 years ago

The fact that one of waitAMinute's arguments is a function is a clear indication that it's an asynchronous function

Is it? Couldn't I have a function that takes another function as an argument, but operates synchronously? I'm sure some lodash functions do that. Eve if they don't I could write one, surely?

Agreed to someone who has some experience with JS this 'looks like' an async function, but the only way to find out for sure is to try it out / look at the sourcecode / docs, isn't it? Or am I wrong somewhere?

zeke commented 8 years ago

Function renamed here: https://github.com/maxogden/art-of-node/pull/65