Convenience functions for lifting to OptionT & EitherT

[henryxparker]

Adding some extension methods that I often wish I had. It really boils down to code cleanliness preferences.

I just wanted to get some feedback on whether others feel like these conversions belong here, or if they're fundamentally anathema/broken in some non-obvious way.

If others are amenable then I will write some tests and correct the formatting

[benhutchison]

Hi @henryxparker Needs clearer motivation : can you include an example usage demonstrating how these extensions make user code cleaner or more concise?

[henryxparker]

Sure! let's say I'm writing a simple program that does a few things but I need to recover from failures. it would look like

def program =
  for {
    requestedThing <- EitherT(getRequestedThing.attempt)
    id <- EitherT.fromOption[IO](parseId(requestedThing.idString), invalidIDError)
    ...
  } yield result

The first thing you read for each line is a bulky EitherT conversion of some sort, and it's not the most readable so I tend to put the right side of each line in its own function so it's already an EitherT when we get there. That cleans it up a bit:

def program =
  for {
    thing <- getThing
    id <- parseId(thing.idString)
    ...
  } yield result

Now we have a nice semantic program, but really I've only pushed it down a bit

def getTheThing: IO[A]

def validate(a: A): Either[E,A]

def getAThingAndValidate: EitherT[IO, E, A] =
  EitherT(
    getTheThing.map(validate)
  )

My example was admittedly small enough to fit onto one line, but it's not hard to imagine a scenario with lots of .zipWithIndex's and .filter's and whatnot extending the length of the line. Regardless of the size, the EitherT is still the first thing you see, despite happening logically after the stuff inside.

I don't think that's a particularly bad thing in most cases, but in this case I find the following notation much neater

def getAThingAndValidate: EitherT[IO, E, A] =
    getTheThing
      .map(validate)
      .liftToEitherT

[danicheg]

Syntaxes for F[Option[A]] and F[Either[L, R]] are aimed to not deal with monad transformers. I'm convinced that methods could be replaced with constructors (smart constructors) of monad transformers themselves.

[danicheg]

cats contains the same method in the Option syntax https://github.com/typelevel/cats/blob/main/core/src/main/scala/cats/syntax/option.scala#L368

[BenHutchisonSeek]

• 👍 to liftEitherT. I found an enrichment in a codebase at work doing basically the same thing, so there's an example of "convergent evolution" and it's also a documented use case in cats. I do agree that OO-style postfix invocations often read nicer, lots of precedents in mouse.

• Approval is contingent on a unit test demonstrating the intended use case. With your motivation piece above ☝️, I was hoping to see compilable code over a "code sketch", as often the usefulness of enriched methods requires type inference to work correctly. Not something we can know in our heads while reviewing a PR. For example, does this version with inferred return type work?

def getAThingAndValidate =
    getTheThing
      .map(validate)
      .liftToEitherT

That's one reason why unit tests matter even for simple methods; they're testing the compiler types.

• Tending towards 👎 on toRightT and toLeftT. They effectively fuse converting an Option to Either, then lifting to EitherT. Im unconvinced the fusing of these, already well supported separately, is common enough. Every extra method we pimp onto the F[A] namespace slows down compilation, IDEs, increases CPU demand while coding, more options to wade through in code completion. Its a small cost but it adds up over time, so we need to be sparing.

• Im 👍 to liftOptionT if we're confident it doesnt already exist in postfix form anywhere, same reasoning as EitherT.

[henryxparker]

I added some tests, formatted, and removed everything except liftOptionT and liftEitherT

[benhutchison]

Thanks for your contribution @henryxparker