scalaz / ioeffect

An effect monad for Scalaz 7.2
Other
55 stars 9 forks source link

MonadIO instances + redeem #55

Closed emilypi closed 6 years ago

emilypi commented 6 years ago

Here are the MonadIO instances available to us, as well as a shameless theft of redeem from ZIO

emilypi commented 6 years ago

@fommil I know you have a special way you'dlike to do instances, feel free to reconfigure, or give a run down of what you thinkis best

fommil commented 6 years ago

Cool!

In 7.2 the MonadIO is able to lift nested monad transformers, did you look into how it is able to do that?

It might be worthwhile overriding a few more things for performance. Especially map.

That said, I prefer the implicit Monad approach. Would it be hard to change that?

emilypi commented 6 years ago

@fommil naw i could change it relatively easily - it just means i get to do less work in the instance creation 😄

fommil commented 6 years ago

also, shouldn't this be LiftIO not MonadIO ?

I don't understand the point of MonadIO.

emilypi commented 6 years ago

@fommil LiftIO is what cats.effect renamed MonadIO. https://hackage.haskell.org/package/transformers-0.2.2.1/docs/Control-Monad-IO-Class.html

I'd rather be at parity with Haskell than further their poor naming conventions

fommil commented 6 years ago

we have prior art https://github.com/scalaz/scalaz/blob/series/7.3.x/effect/src/main/scala/scalaz/effect/LiftIO.scala (going back to 2011, at least)

fommil commented 6 years ago

I don't think this needs an implicit Monad to implement.

I think the E should be on the typeclass rather than the method.

emilypi commented 6 years ago

@fommil It looks like they made LiftIO MonadIO, and MonadIO Monad with LiftIO. I don't know about that, since MonadIO requires monad laws to make sense as a typeclass. There really isn't a semistructure here to abstract out. It's one method

emilypi commented 6 years ago

@fommil alright I've switched it up so we're no longer using inheritance, but now the implicits aren't being found for each transformer appropriately. Would this have something to do with the new wait we're making instances discoverable? This should compile.

fommil commented 6 years ago

if you want laws, you'll need to extend Monad and add them using the scalaz style, e.g. see how Monad does it from the Bind laws.

fommil commented 6 years ago
[error] MonadIOInstances.scala:16:25: type parameter E defined in method liftIO shadows type E defined in method ioMonadIO. You may want to rename your type parameter, or possibly remove it.
[error]     override def liftIO[E, A](io: IO[E, A])(implicit M: Monad[IO[E, ?]]): IO[E, A] = io
[error]                         ^
emilypi commented 6 years ago

Will do. I'll get this fixed up by tonight.

emilypi commented 6 years ago

@fommil alright, looks like we're good to go on this one