getify / Functional-Light-JS

Pragmatic, balanced FP in JavaScript. @FLJSBook on twitter.
http://FLJSBook.com
Other
16.6k stars 1.96k forks source link

Some issue I encountered #138

Closed JoeHetfield closed 6 years ago

JoeHetfield commented 6 years ago

Hi, Kyle

While I updating the Chinese translation, I encountered some small problem. Perhaps some editorial issue:

  1. ch3.md line 149:

Warning: Although the () => p2 arrow function version is shorter than constant(p2), I would encourage you to resist the temptation to use it. The arrow function is returning a value from outside of itself, which is a bit worse from the FP perspective. We'll cover the pitfalls of such actions later in the book (see Chapter 5, "Reducing Side Effects").

At the last of this paragraph, I think there should be a link to the actual location, just like other place where cross reference happened.

  1. ch3.md line 520:

The three sets of (..)s denote three chained function calls. But perhaps splitting out each of the three calls helps see what's going on better:

"The three sets of (..)" -- I didn't spot where are they. Maybe you planned to write the previous code snippet like:

curriedAjax(..)(..)(..);

and then changed your mind?

I will keep posting the issue I found.

JoeHetfield commented 6 years ago

ch3.md line 630:

// now with currying: // (5 to indicate how many we should wait for) var curriedSum = curry( sum, 5 );

I think it might be:

// now with currying: // (5 to indicate how many arguments we should wait for) var curriedSum = curry( sum, 5 );

getify commented 6 years ago

I think there should be a link to the actual location

Yep, just missed it. Should just be "Chapter 5" and link to ch5.md. Fixed in ebe1aab22e7493cca977352096136a2524157245.

"The three sets of (..)" -- I didn't spot where are they.

Immediately above that sentence, in this code snippet:

curriedAjax( "http://some.api/person" )
    ( { user: CURRENT_USER_ID } )
        ( function foundUser(user){ /* .. */ } );

I think it might be:

Eh, could be changed, but I don't see any necessity to do so.

getify commented 6 years ago

BTW, just posted a few more tweaks (fixing bugs in code, some wording clarifications), including adding in another missed cross-ref. Make sure to check these commits out:

35e1775e817d0296db9be58923f23f6cc29f2466

536514500fcb5ec863977b7298e3c167caa2732c

JoeHetfield commented 6 years ago

Sure, will keep eye on them

JoeHetfield commented 6 years ago

ch4.md, The candy factory metaphor, line 90-92

To keep up with demand for more candy production, they decide to take out the conveyor belt contraption and just stack all three machines on top of one another, so that the output valve of one is connected directly to the input valve of the one below it. There's no longer sprawling wasted space where a chunk of chocolate slowly and noisily rumbles down a conveyor belt from the first machine to the second.

This innovation saves a lot of room on the factory floor, so management is happy they'll get to make more candy each day!

The logic here seems strange to me:

new machine --> space saved --> more candy produced

I think the saved space is not relate to the productivity directly... I'd rather consider about it like this:

new machine --> space saved --> more machine moving in --> more candy produced

but this seems strange on the code side as well:

function composition --> code get cleaner --> more code --> good result for coder?

Finally, I think this one make the most sense to me:

function composition --> code get cleaner --> easy to work with --> satisfied coder

so, maybe the original candy factory metaphor could be:

new machine --> space saved --> easy to work with --> happy empolyees/managers

JoeHetfield commented 6 years ago

ch4.md line 713 & 737:

Seems missed the code type annotation "js"

getify commented 6 years ago

Sorry to add more work to your updates, but I had to make a bunch more tweaks/fixes while preparing the book for a print layout. They're all merged in this one commit:

0ddc4a7bf198a92b7d1baf24d93f233d5c8c1bd6

JoeHetfield commented 6 years ago

It's OK, man. I will keep following

getify commented 6 years ago

Couple more small tweaks:

411fc4a459eac900f7f77652c68144bd1b8cc612

ac81c0d0019ba7ebc3bcaa227f3c25a65717dba4

JoeHetfield commented 6 years ago

ch8.md line 220,245,272

Seems missed the code type annotation "txt"

getify commented 6 years ago

thanks, fixed (all 3): c1d3e176125395111a298347cf57f7e8f6fbef08

JoeHetfield commented 6 years ago

ch9.md line 1240

Since the graph been moved to inlined with the text, the last colon of this paragraph shoud be a period.

JoeHetfield commented 6 years ago

ch9.md line 1481

Seems missed a blank line

JoeHetfield commented 6 years ago

ch11.md line 111

Seems missed a crossref to chapter 3.

getify commented 6 years ago

thanks, found and fixed several more missing crossrefs (see f883f55180ab2df067026cc474aa3000e4b6101b and c2cc588ef90d5d7b97e26aaae11a6a85d9e9ab74)

JoeHetfield commented 6 years ago

apB.md line 127

as chain(inspect) accomplishes the same goal;

I think it should be:

as chain(identity) accomplishes the same goal;

getify commented 6 years ago

Good catch!

JoeHetfield commented 6 years ago

apB.md line 206

Maybe is a monad that either holds a Just or an Empty.

I think it should be:

Maybe is a monad that either holds a Just or a Nothing.

getify commented 6 years ago

see: f59f9fa4ff07c46dc398de09b7925ddd2816b9ae (for bug fix of code in ch5)

JoeHetfield commented 6 years ago

apB.md line 260

why is Maybe useful at all?!?

I think the first question mark is not necessary:

why is Maybe useful at all!?

getify commented 6 years ago

eh, it's not necessary, but it's informal to show intensity.

getify commented 6 years ago

closing for now since it seems we addressed all this feedback.

YagamiNewLight commented 5 years ago

I'm sorry if I bothered you two guys, In the ch3.md line 146:

p1.then( foo ).then( constant( p2 ) ).then( bar );

I tried to chain my promises using this way but I failed. Later I found out the async/ajax(p2 here) action was fired when the constant fn invoked the p2. So the p2 and p1 are fired at the same time. Maybe constant function for chaining promise can be revised like this

function thenable(fn,...args){
  return function(){
     return fn(...args)
  }
}

In this way, the promises can be chained corretly.