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

Added additional definitions from Yampa to BearRiver as discussed in … #63

Merged
merged 2 commits into from
Dec 4, 2017
Merged

Conversation

thalerjonathan
Copy link
Contributor

…in #40: never, now, once and >--

@@ -143,6 +143,24 @@ hold a = feedback a $ arr $ \(e,a') ->
loopPre :: Monad m => c -> SF m (a, c) (b, c) -> SF m a b
loopPre = feedback

-- | Event source that never occurs.
never :: Monad m => SF m a (Event b)
never = MSF $ \_ -> return (NoEvent, never)
Copy link
Owner

Choose a reason for hiding this comment

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

never = constant NoEvent

In general it's best not to use internal constructors if possible.

Using the API, as opposed to the internal constructors, facilitates changing the library internally without affecting uses/users. This is part of the reason for an API in the first place.

While this often creates some (small) performance loss, it's worth it when libraries grow big, since you can change the internals quickly. For example, if you choose to use GADTs to improve performance, and implement constant using that optimisation, you'll only get a gain if you stuck to the API. For another example, if you choose to add another field or change the order of the elements in the tuple, you won't have to change everything.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, I understand.
Saw that you use the internal MSF constructor a couple of times and assumed it is OK in such cases. But of course when a function can be implemented using the existing API then it should always go that way - but sometimes it is not clear for me to see that but thanks for the help.

(>--) :: Monad m => a -> SF m a b -> SF m a b
a0 >-- sf = MSF $ \_ -> do
(b, ct) <- unMSF sf a0
return (b, ct)
Copy link
Owner

@ivanperez-keera ivanperez-keera Dec 4, 2017

Choose a reason for hiding this comment

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

This could be factorized:

replaceOnce :: a -> MSF a a
replaceOnce x = MSF $ \_ -> return (x, id)

(-->) :: Monad m => b -> SF m a b -> SF m a b 
b0 --> sf = sf >>> replaceOnce b0

(>--) :: Monad m => a -> SF m a b -> SF m a b 
a0 >-- sf = replaceOnce a0 >>> sf

The question is: can one write replaceOnce without using unMSF? It's possible, using switching :)

EDIT: the implementation of replaceOnce was incorrect. I've fixed it.

Copy link
Owner

Choose a reason for hiding this comment

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

@turion You've implemented a new exception-handling mechanism. What's the current way of writing \x -> dSwitch (constant (x, Event)) (\_ -> id) using that API?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Awesome :)
Shouldn't replaceOnce look like:

replaceOnce :: Monad m => a -> MSF m a a
replaceOnce x = MSF $ \_ -> return (x, arr id)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

And here without using the internal MSF constructor:

replaceOnce :: Monad m => a -> SF m a a
replaceOnce a = dSwitch (arr $ const (a, Event ())) (const $ arr id)

Copy link
Owner

Choose a reason for hiding this comment

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

replaceOnce :: Monad m => a -> SF m a a
replaceOnce a = dSwitch (constant (a, Event ())) (const $ arr id)

?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah obviously ;)

@ivanperez-keera ivanperez-keera merged commit 2706b72 into ivanperez-keera:develop Dec 4, 2017
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.

2 participants