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 DeferredKMonadThrow #1231

Merged
merged 4 commits into from
Jan 5, 2019
Merged

Add DeferredKMonadThrow #1231

merged 4 commits into from
Jan 5, 2019

Conversation

daneko
Copy link
Contributor

@daneko daneko commented Jan 4, 2019

No description provided.

@@ -71,6 +71,9 @@ interface DeferredKMonadError: MonadError<ForDeferredK, Throwable>, DeferredKMon
deferredHandleErrorWith { f(it).fix() }
}

@extension
interface DeferredKMonadThrow: MonadThrow<ForDeferredK>, DeferredKMonadError

@extension
interface DeferredKBracket: Bracket<ForDeferredK, Throwable>, DeferredKMonadError {
Copy link
Member

Choose a reason for hiding this comment

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

interface DeferredKBracket: Bracket<ForDeferredK, Throwable>, DeferredKMonadThrow {

Copy link
Member

Choose a reason for hiding this comment

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

Or maybe this goes in MonadDefer. Please check the other implementations :D

Copy link
Contributor Author

Choose a reason for hiding this comment

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

thinks poiting out, and fixing and check other impl.
DeferredKMonadDeferred already used DeferredKBracket, so I fixed DeferredKBracket using DeferredKMonadThrow.

(compare to io.kt and singlek.kt)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

by the way, I have some question.

  1. DeferredKAsync#invoke is deprecated,
    so can I deleted it in this PR? or make other PR?

  2. Which do you prefer implementation io.kt or singlek.kt (or else) ? (when I implement it)

(Please excuse my poor English and these question are little related this PR.)

Copy link
Member

Choose a reason for hiding this comment

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

For the typeclasses, the derived functions are often less optimal than a concrete version. And thus if a data type has a concrete impl is available, it's always beneficial to override it.

The only important thing is that laws don't break, which is checked by tests (laws).

MonadThrow sits between MonadError and Bracket so that look goods!

Copy link
Member

Choose a reason for hiding this comment

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

About deprecating DeferredKAsync#invoke I am fine with removing it, but it's probably best to keep that for another PR. That way you'll also have the joy of closing and contributing 2 PRs ;)

About your second questions.
It seems that the SingleK instance is incorrect, since MonadError should be a valid instance for both Monad and ApplicativeError. Which is not the case for SingleK.

interface SingleKMonadError : MonadError<ForSingleK, Throwable>, SingleKMonad

As you can see it does not implement ApplicativeError<ForSingleK, Throwable> (SingleKApplicativeErrorInstance).

This is the reason SingleK implements the applicative-error methods here.

The reason io (re)implements applicative methods is because they're both implemented by Monad & ApplicativeError and thus need to explitely redefine it to avoid the diamond problem

Copy link
Member

@pakoito pakoito left a comment

Choose a reason for hiding this comment

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

Nicely done, thanks ;)

@pakoito pakoito merged commit 0539511 into arrow-kt:master Jan 5, 2019
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