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

schedSelectClock should only use a Semigroup #42

Closed
turion opened this issue May 9, 2018 · 7 comments
Closed

schedSelectClock should only use a Semigroup #42

turion opened this issue May 9, 2018 · 7 comments

Comments

@turion
Copy link
Owner

turion commented May 9, 2018

With GHC 8.4, Semigroups become more of a thing, so it's time to relax the type signature of schedSelectClock to requiring cl only to be a Semigroup.

@turion turion changed the title schedSelectClock should only use a SemiGroup schedSelectClock should only use a Semigroup May 9, 2018
@turion
Copy link
Owner Author

turion commented May 9, 2018

We support GHC >= 8, so I think no conditional imports are needed.

@SolviQorda
Copy link
Contributor

So I had a go at this - and the following compiles. Have I understood what you were asking for in the issue?

schedSelectClocks
  :: (Monad m, Semigroup cl, Clock m cl)
  => Schedule m (SelectClock cl a) (SelectClock cl b)
schedSelectClocks = Schedule {..}
  where
    startSchedule subClock1 subClock2 = do
      (runningClock, initialTime) <- startClock
        $ mainClock subClock1 <> mainClock subClock2
      let
        runningSelectClocks = concatS $ proc _ -> do
          (time, tag) <- runningClock -< ()
          returnA                     -< catMaybes
            [ (time, ) . Left  <$> select subClock1 tag
            , (time, ) . Right <$> select subClock2 tag ]
      return (runningSelectClocks, initialTime)

@turion
Copy link
Owner Author

turion commented May 28, 2018

Yes, that's the idea! Feel free to apply this to all other schedules with a Monoid requirement and send a PR. (Use git grep Monoid)

@SolviQorda
Copy link
Contributor

I've managed to get all but one of the Schedules onto Semigroups and compile.

My only problem is in Concurrently.hs, with concurrentlyWriter. I'm trying to find a way for Semigroups to work with Monad Transformers.

I'm happy to keep trying with concurrentlyWriter, or to send a PR with all the changes except concurrentlyWriter --> whatever is more useful!

@turion
Copy link
Owner Author

turion commented May 29, 2018

Ah, the Monoid in concurrentlyWriter is of a different nature! Have a look at The Monad instance for WriterT. It requires Monoid w, and that's the sole reason for requiring it in concurrentlyWriter.

@turion
Copy link
Owner Author

turion commented May 29, 2018

So please do send the PR with everything you have so far :)

@SolviQorda
Copy link
Contributor

Ah I thought that might be the case - glad that I didn't spend hours trying to refactor something I couldn't 😂

@turion turion closed this as completed May 29, 2018
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