typelevel/cats-mtl

Add revised MonadLog?

vlovgr opened this issue · 10 comments

vlovgr commented

If we adapt the MonadLog type class presented in IO & Logging Capabilities to fit as an extension to FunctorListen, we end up with the following. We've replaced log with tell and extract is replaced by listen.

trait MonadLog[F[_], L] extends FunctorListen[F, L] {
  val monad: Monad[F]

  val monoid: Monoid[L]

  def clear[A](fa: F[A]): F[A]

  def flush[A](fa: F[A])(f: L => F[Unit]): F[A]
}

Personally, I don't think it's the right way to go:

  • we're extending from a Functor hierarchy, while we require Monad,
  • this makes us require not one, but two additional constraints, with Monoid,
  • logs are forced to take a form like F[(A, L)], and cannot be stored separately.

Instead, I propose we adapt MonadLog as an extension to MonadState. In fact, any MonadState[F, L] can be a MonadLog[F, L] with a Monoid[L] instance, meaning we can reuse the existing MonadState instances. Such an adapted version of MonadLog could be defined as follows.

trait MonadLog[F[_], L] extends MonadState[F, L] with Serializable {
  val monoid: Monoid[L]

  def log(l: L): F[Unit]

  def clear: F[Unit]

  def flush(f: L => F[Unit]): F[Unit]
}

Default implementation could be defined as follows.

trait DefaultMonadLog[F[_], L] extends DefaultMonadState[F, L] with MonadLog[F, L] {
  def log(l: L): F[Unit] =
    modify(monoid.combine(_, l))

  def clear: F[Unit] =
    set(monoid.empty)

  def flush(f: L => F[Unit]): F[Unit] =
    monad.flatMap(monad.flatMap(get)(f))(_ => clear)
}

Laws could be defined as follows.

trait MonadLogLaws[F[_], L] extends MonadStateLaws[F, L] {
  implicit val logInstance: MonadLog[F, L]
  implicit val monoid: Monoid[L] = logInstance.monoid

  import logInstance._
  import monad.{pure, unit}
  import monoid.empty

  def logThenClearDoesNothing(l: L) =
    log(l) *> clear <-> unit

  def logThenGetReturnsLog(l: L) =
    log(l) *> get.flatMap(clear.as) <-> log(l) *> clear.as(l)

  def logsThenGetReturnsCombinedLog(l1: L, l2: L) =
    log(l1) *> log(l2) *> get.flatMap(clear.as) <->
      log(l1) *> log(l2) *> clear.as(l1 combine l2)

  def clearThenGetReturnsEmpty =
    clear *> get <-> pure(empty)

  def clearThenClearClearsOnce =
    clear *> clear <-> clear

  def flushIsGetThenClear(f: L => F[Unit]) =
    flush(f) <-> get.flatMap(f) *> clear
}
LukaJCB commented

Hey @vlovgr, thanks for raising this issue. I'm not sure I fully understand everything just yet, I'm going to try to read the blog post as soon as possible :)

vlovgr commented

@LukaJCB thanks for taking the time! I'm just looking to get some feedback on whether this would be a useful addition to the project or not. :) I realised I haven't properly explained why MonadLog looks like it does, so thought I would provide a further explanation below.

If you're using FunctorListen for log accumulation, you basically don't have the ability to clear the accumulated logs. This is important if you ever want to not dispatch all the logs at the same time. For example, whilst accumulating logs, you might want to immediately dispatch a single log message. Or whilst processing a request, you want to accumulate some log entries, then dispatch those as a message, and then keep going without those entries.

The MonadLog version in the blog post, and the revised version extending FunctorListen at the top of this issue, basically adds clear to let you do just that. To be able to write meaningful laws for clear, we need to know what empty looks like, hence the requirement on a Monoid instance. If we have Monoid, then we can write further laws for log and extract (i.e. tell and listen), which explicitly describe monoidal log accumulation.

If you, at some point, want to dispatch currently accumulated log entries (even if a single one), you'd have to write something like the following, using listen and clear. The main point to note is that it requires flatMap, and hence Monad (technically FlatMap would do).

F.clear {
  for {
    aWithLogs <- F.listen(fa)
    (a, logs) = aWithLogs
    _ <- dispatch(logs)
  } yield a
}

If we know we'll always need to have a Monad to support this intermediate dispatch, then we could just as well require it in MonadLog, and support it with a flush operation.

For the reasons listed in the issue description, I don't think MonadLog extending FunctorListen is the way to go. The other option I propose is to extend MonadState. Here we essentially make MonadState more strict, by requiring a Monoid, and write laws to describe how monoidal log accumulation happens. The benefit over MonadState is just that: log accumulation laws, and the reusability that comes with it.

With the MonadState version we're also free to implement log accumulation both as F[(A, L)] (e.g. StateT or WriterT), or with e.g. a Ref from cats-effect. In a way, it's like a combination of a more powerful FunctorListen and MonadState.

LukaJCB commented

It's very interesting, I'm probably going to have to think about it more, but I think MonadState might be over-constraining, writing a clear function for WriterT is completely doable and IIUC you could write a MonadLog instance for WriterT as well. For flush it depends on the version we want to support. WriterT can easily support def flush[A](fa: F[A])(f: L => F[Unit]): F[A], but obviously can't support def flush(f: L => F[Unit]): F[Unit].

I'm a bit wary of adding type classes that have too much power when one could achieve the same thing with more limited requirements.

That said, if we look at the Haskell or PureScript mtl libraries, we'll see that their MonadWriter actually has an additional method to listen and tell: pass, which looks like this:

class MonadTell w m <= MonadWriter w m | m -> w where
  listen :: forall a. m a -> m (Tuple a w)
  pass :: forall a. m (Tuple a (w -> w)) -> m a

With pass and Monoid[L], you could easily define clear.
Another thing you could do instead of using MonadState is to make use of Group:

def clear[F[_]: FlatMap, L: Group](fa: F[A])(implicit F: FunctorListen[F, L]): F[A] =
  F.listen(fa).flatMap { case (a, l) => F.tell(Group[L].inverse(l)).as(a) }

Though this is probably less useful in practice, since there's not many Group instances out there and no collection type currently supports it (though it would be interesting to try to develop one), so this is probably not the best way to go.

Instead, I'd like to make a counter proposal, how about an ApplicativePass:

trait ApplicativePass[F[_], L] extends FunctorListen[F, L] {
  val applicative: Applicative[F]
  val monoid: Monoid[L]
  override val functor: Functor[F] = applicative

  def pass[A](faf: F[(A, L => L)]): F[A]

  def clear[A](fa: F[A]): F[A] =
    pass(functor.tupleRight(fa, _ => monoid.empty))
}

As for laws, we can use the laws from your blog post in addition to what I could think of for pass:

pass(pure(a -> f)) === tell(f(Monoid[L].empty)) *> pure(a)
pass(tell(l).tupleRight(f)) === tell(f(l))

listen(pass(pure(a -> f))) === pure(a -> f(Monoid[L].empty))
listen(pass(tell(l).tupleRight(f)) === pure(() -> f(l))

And we can also add additional laws for tell given the monoid constraint:

tell(l1) *> tell(l2) === tell(l1 |+| l2)
tell(l1) <* tell(l2) === tell(l1 |+| l2)

WDYT?

vlovgr commented

Thanks a lot! That looks very interesting. I agree with all your points. However, my main worry is that clear without flush (ideally def flush(f: L => F[Unit]): F[Unit], as I'm interested in using a Ref) is not as useful on its own. In my experience, I actually always want flush rather than clear (clear just being a consequence of using flush). If we had MonadWriter, I think that would've been a good base for MonadLog, simply requiring Monoid.

LukaJCB commented

Okay, so I think we might need to divide between what's convenient for the end user and what the base abstraction is supposed to express.

Instead of using MonadWriter and forcing Monad even though the laws don't need it, maybe we could add some syntax for flush that requires a Monad:

trait PassSyntax {
  implicit def toPassOps[F[_], A](fa: F[A]): PassOps[F, A] = new PassOps(fa)
}

final class PassOps[F[_], A](val fa: F[A]) extends AnyVal {
  def clear[L]: F[A](implicit A: ApplicativePass[F, L]): F[A]
  def flush[L](fa: F[A])(f: L => F[Unit])(implicit F: Monad[F], A: ApplicativePass[F, L]): F[A]
}

I think this may be more in line with the spirit of this project :)

vlovgr commented

Yes, I totally agree ApplicativePass is better than MonadLog based off MonadState. And yes, you only need Monad for an internal law, so I think what you suggest is perfectly fine here. 👍 I'm fairly new to the project, so really appreciate the guidance. :)

LukaJCB commented

Cool! Would you be willing to create a PR for this? :)

I'd also like to hear from the other maintainers /cc @SystemFw @andyscott

vlovgr commented

I think ApplicativePass looks neat, but I'm mainly interested in having the log accumulation happening outside of F[A] (e.g. in a Ref), rather than with e.g. a WriterT. That is, I'm still looking to support the following functions (whether flush is on the type class or not):

def clear: F[Unit]
def flush(f: L => F[Unit]): F[Unit]

and, as far as I can tell, ApplicativePass cannot support those. The only place I can see these be supported with the current type classes is via MonadState. (I could be missing something, please let me know if that's the case.) Hence, I'm not that inclined to write up ApplicativePass. I'm more than happy to keep a MonadLog version based of MonadState in my own project, if the feeling is it doesn't belong here. That is also perfectly fine. :)

I still think ApplicativePass would be a nice addition to the project. Should we rename this issue to 'Add ApplicativePass', and whoever is interested in contributing it, can do so? :)

LukaJCB commented

Yeah if you want a signature like def flush(f: L => F[Unit]): F[Unit] without any input F[A] you're going to need something context-sensitive like State. For the less restrictive signature def flush[L](fa: F[A])(f: L => F[Unit]): F[A]

I'm fully on board with log accumulation happening outside of F[A] instead of using WriterT. That should work with either ApplicativePass and MonadState, I reckon.

That said, I'm not entirely sure this project would be the best place for MonadLog, it seems to just be a tiny wrapper around MonadState that IMO, isn't general enough to be included, though I'm open to change my mind if given convincing arguments :)

vlovgr commented

Yes, MonadLog is just that: the addition of a Monoid requirement, and laws for how log accumulation happens, so it can be reused. I can see how it makes sense to only add type classes that are general enough. It would be good to try and define 'general enough' as advice for future contributors as well. :)

I'm going to close this. Feel free to create a new issue for ApplicativePass. :)