Constraining the type of cmapM_

[tomsmalley]

There are two similar functions:

cmapM :: forall w m cx cy. (Get w m cx, Set w m cy, Members w m cx) => (cx -> SystemT w m cy) -> SystemT w m ()
cmapM_ :: forall w m c a. (Get w m c, Members w m c) => (c -> SystemT w m a) -> SystemT w m ()

I can see the reasons for why cmapM_ has this type - to match mapM_ - but if you intended to use cmapM but wrote cmapM_ accidentally, there is nothing to warn you that your properties won't get set:

cmapM_ $ \(Velocity v) -> do
  liftIO $ print v
  pure $ Velocity (v - 1)

This compiles fine. I propose constraining it to

cmapM_ :: forall w m c. (Get w m c, Members w m c) => (c -> SystemT w m ()) -> SystemT w m ()

so now it's impossible to make mistakes like above. I think this is a valid change: cmapM already deviates from mapM by returning (), so maintaining a close match to those types doesn't seem so important.

[jonascarpay]

I think you make a good point, and I'm willing to try it out, PR's are welcome. Since it's an API-breaking change, it would need to go on the v0.9 branch, which I want to merge closer to the release of lts-15.

[jonascarpay]

Fixed in 103daa1

[tomsmalley]

Oh, thanks, I'd intended to create a PR but forgot about this one