Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix various errata in Async & Performance Ch. 4: Generators #434

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

airandfingers
Copy link
Contributor

I could be wrong/crazy, but isn't this change (062b377) correct?

Step 9. The first request() call should be in the *foo(1) instance, as *foo(2) and *foo(3) are waiting.

Step 10. *foo(2) yields its request promise back to *foo(3), not to *foo(1), which has already completed by this time.

I could be wrong/crazy, but isn't this change correct?

9. The first request() call should be in the *foo(1) instance, as *foo(2) and *foo(3) are waiting.

10. *foo(2) yields its request promise back to *foo(3), not to *foo(1), which has already completed by this time.
The thunkify definitions as published will only work the first time they are called, after which the thunkory will continue appending to a shared args array, calling `fn` with more and more arguments.

To prevent this, the thunkory should clone a new copy of args every time it is called, and push the latest cb to that args array.
The explanation of the process() manual transformation function includes references to a "resolve" function that is not used in the example. 

It's clear from context that the explanation is referring to the "yield request" statement replaced by "return request".
@airandfingers airandfingers changed the title Swap *foo(1) and *foo(3) in delegated yield steps 9 and 10 Fix various errata in Async & Performance Ch. 4: Generators May 9, 2015
@airandfingers
Copy link
Contributor Author

My second commit (594669c) fixes a clear issue: as written, thunkify returns a thunkory that only works the first time it is called. Subsequent attempts will most likely call the callback given to the thunkory in its first call (depending on the implementation of fn).

It's worth reviewing, though, as I am changing the way a code example works.

@airandfingers
Copy link
Contributor Author

My third commit (8b5c7df) fixes an obvious typo, a reference to "resolve" that should be "request".

@getify
Copy link
Owner

getify commented May 9, 2015

I can make the change in 8b5c7df as an errata change to the existing first edition. The changes in 594669c need to hold off to second edition. The changes in 062b377 are only partially correct, there are other problems with that list of steps that I need to correct. So I will have to fix that whole list in the second edition.

@AnatoliiKolesnyk
Copy link

AnatoliiKolesnyk commented Jan 19, 2018

@airandfingers, you missed replacing *foo(3) --> *foo(1) at the very end of the 8 clause (line 1519)

Also, if I am not mistaken, at clause 10, val variable of *foo(3) is used and not of *foo(2):

When the promise resolves, the second Ajax response propagates all the way back into the *foo(2) generator instance, and is assigned to its local val variable.`

@OahMada
Copy link

OahMada commented Aug 9, 2020

Why I run the existing thunkify code and it works just fine?

@getify
Copy link
Owner

getify commented Aug 9, 2020

Here's an illustration of how the book's version of thunkify(..) utility is "broken":

function thunkify(fn) {
	return function() {
		var args = [].slice.call( arguments );
		return function(cb) {
			args.push( cb );
			return fn.apply( null, args );
		};
	};
}

var f = thunkify((x,y,cb) => cb(x + y));

var seven = f(3,4);

seven(sum => console.log(`Seven: ${sum}`));    // Seven: 7
seven(sum => console.log(`Another Seven: ${sum}`));    // Seven: 7     <---- Oops
seven();   // Seven: 7    <------- WHAT!?

We would have expected the next-to-last line to print Another Seven: 7, but it didn't. The shared closure over args modified the list in the first seven(..) call to add the first version of the callback. From then on, all subsequent calls to seven(..) don't pay any attention to what callback you pass in.

Compare to the corrected thunkify(..):

function thunkify(fn) {
	return function() {
		var args = arguments;
		return function(cb) {
			var thunk_args = [].slice.call( args );
			thunk_args.push( cb );
			return fn.apply( null, thunk_args );
		};
	};
}

var f = thunkify((x,y,cb) => cb(x + y));

var seven = f(3,4);

seven(sum => console.log(`Seven: ${sum}`));     // Seven: 7
seven(sum => console.log(`Another Seven: ${sum}`));     // Another Seven: 7

So, yes, the book's version is broken and that's regrettable. But it was enough of a substantive change (and would have needed corrective text to explain), and the book had already been out for quite awhile at that point... so I opted not to modify the book. At some point, a second edition will be written, and this issue will be addressed then.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Second Edition
Async & Performance
Development

Successfully merging this pull request may close these issues.

None yet

4 participants