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

Add Async crock to Documentation #210

Merged
merged 2 commits into from
Feb 15, 2018
Merged

Add Async crock to Documentation #210

merged 2 commits into from
Feb 15, 2018

Conversation

evilsoft
Copy link
Owner

Yet Another

image

This PR adds the Async README to the documentation. That is all.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 100.0% when pulling 851f625 on move-async-docs into 85d549a on master.

1 similar comment
@coveralls
Copy link

Coverage Status

Coverage remained the same at 100.0% when pulling 851f625 on move-async-docs into 85d549a on master.

@coveralls
Copy link

coveralls commented Feb 14, 2018

Coverage Status

Coverage remained the same at 100.0% when pulling 2d6cff1 on move-async-docs into 85d549a on master.

Copy link
Collaborator

@karthikiyengar karthikiyengar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've had a first run at this - I'll do another scan if I find time :-)

const repeat =
char => n => char.repeat(n)

// values :: Async String Number -> Async String Number
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm still pretty new to reading signatures, but this doesn't feel right? Did you mean values :: Async String Number -> Async Number String?

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nope, that siggy is correct as is. In this case we are map values to their respective types:

// repeat :: String -> Number -> String
// length :: String -> Integer

so the Rejected which runs through length, will take the String and deposit a Number in the Resolved and the repeat('a') will take a Resolved Number and map it to a String before dumping it into a Rejected.

Using the functions to allow mapping keep from the .swap().bimap(repeat('a'), length) which for me at least occurs more often then just a straight .swap(). So the crocks swaps allow for mapping values.

If that makes sense?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, thanks for the clarification :-)

)

// fake :: String -> Async String Object
const fake = composeK(
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This example assumes that the reader is already familiar with what composeK does. Do you think being more explicit would help?

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I put function like that in these examples as teasers, do people will be like what is this composeK and then go look it up. Like reading a book with a Dictionary, do you think it would be more helpful to the reader to do this instead:

const fake = compose(
  chain(lookup),
  chain(safe(test(/^file-(a|b|c)/)),
  chain(safe(isString))
)

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, we can retain the composeK. The only reason I felt that way because the section header was chain but there was no reference to it in the code snippet.

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That is an excellent point! Good call! I will make that change for this one, and leave the other one as composeK, which will give a hint as to what the heck composeK could mean. What do you think?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, I think that makes sense.

@evilsoft
Copy link
Owner Author

image

@evilsoft evilsoft merged commit 79510b9 into master Feb 15, 2018
@evilsoft evilsoft deleted the move-async-docs branch February 15, 2018 01:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants