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 preconditions to -mbind implementations #151

Closed
yurrriq opened this issue Feb 20, 2016 · 9 comments
Closed

Add preconditions to -mbind implementations #151

yurrriq opened this issue Feb 20, 2016 · 9 comments

Comments

@yurrriq
Copy link
Collaborator

yurrriq commented Feb 20, 2016

Per #150, et al., mlet can be a little confusing wrt context. In an effort to fail faster (and not wait for #140), we should add preconditions to ensure the proper context in each -mbind implementation.

@yurrriq
Copy link
Collaborator Author

yurrriq commented Feb 20, 2016

iirc protocol method implementations can't have :preconditions, but a simple if that throws an error upon encountering a context mismatch should suffice.

@niwinz
Copy link
Member

niwinz commented Jun 28, 2016

Working on it on #173

@niwinz niwinz self-assigned this Jun 28, 2016
@niwinz
Copy link
Member

niwinz commented Jun 29, 2016

What do you @yurrriq think about this:

(require '[cats.context :as ctx] :reload)
(require '[cats.core :as m] :reload)
(require '[cats.monad.either :refer [left right]])
(require '[cats.monad.maybe :refer [just nothing]])

(m/mlet [a (just 1) b (right 2)] (m/return (+ a b)))
;; => #<Just 3>

(alter-var-root #'ctx/*strict* (constantly true))
;; => true

(m/mlet [a (just 1) b (right 2)] (m/return (+ a b)))
;; => IllegalArgumentException Context mismatch: you can't mix different context.  cats.context/throw-illegal-argument (context.cljc:33)

@niwinz
Copy link
Member

niwinz commented Jun 29, 2016

It not yet finished (not in master) because I need some fixes before but is a generic approach instead of having assert on each -mbind impl. It just checks if the previously set context is equal to the current provided one and if they mismatches, then the exception is raised if strict mode is enabled.

@niwinz niwinz closed this as completed Jun 29, 2016
@yurrriq
Copy link
Collaborator Author

yurrriq commented Jun 29, 2016

I think the first example should always fail. The check and error message seem solid, though. What else does strict mode do?

@niwinz
Copy link
Member

niwinz commented Jul 3, 2016

Just that, it disallows overwrite the already set context and checks if the contexts are different and just throws an exception. In any case I agree with you and I will add asserts independently of the strict mode.

@niwinz
Copy link
Member

niwinz commented Jul 4, 2016

(m/mlet [a (just 1) b (right 2)] (m/return (+ a b)))
;; AssertionError Assert failed: Context mismatch: #<Right 2> is not allowed to use with maybe context.

This seems like a much better error message and works as is, without strict mode.

@yurrriq
Copy link
Collaborator Author

yurrriq commented Jul 4, 2016

Looks good to me.

@niwinz
Copy link
Member

niwinz commented Jul 4, 2016

Nice, this is already in master ;)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants