Add EitherT docs #1854
Add EitherT docs #1854
Conversation
| val error: EitherT[Option, String, Int] = EitherT.leftT("Not a number") | ||
| ``` | ||
|
|
||
| ## From `F[A]` or `F[B]` to `EitherT[F, A B]` |
There was a problem hiding this comment.
Missing a comma in EitherT[F, A*,* B]
| } | ||
| ``` | ||
|
|
||
| Clearly, the updated code is less readible and more verbose: the details of the |
| ``` | ||
|
|
||
| Note that since `EitherT` is a monad, monadic combinators such as `flatMap` can | ||
| be used to compose `EitherT` values. |
There was a problem hiding this comment.
Maybe clarify here, that EitherT forms a Monad, only if F[_] is also a Monad :)
|
Also your Tut examples seem to fail on 2.11.x and 2.10.x. You can execute |
| Use the `value` method defined on `EitherT` to retrieve the underlying `F[Either[A, B]]`: | ||
|
|
||
| ```tut:book | ||
| val errorT: EitherT[Option, String, Int] = EitherT.leftT("foo") |
There was a problem hiding this comment.
I know it's just an example, but an EitherT[Option, String, Int] doesn't really seem that useful, and I'm not sure if we should include those in the official docs. Maybe change it to EitherT[Future, String, Int] instead? :)
There was a problem hiding this comment.
Yeah, that's much better actually.
Codecov Report
@@ Coverage Diff @@
## master #1854 +/- ##
=======================================
Coverage 94.96% 94.96%
=======================================
Files 241 241
Lines 4173 4173
Branches 106 106
=======================================
Hits 3963 3963
Misses 210 210Continue to review full report at Codecov.
|
There was a problem hiding this comment.
This looks really good @Technius, thanks. I left a few comments.
|
|
||
| ```tut:book | ||
| import scala.util.Try | ||
| import cats.syntax.either._ |
There was a problem hiding this comment.
We decide to use the uber import import cats.implicits._ in the documentation, see issue #1026.
| } yield result | ||
|
|
||
| divisionProgramAsync("4", "2").value // Future(Right(2.0)) | ||
| divisionProgramAsync("a", "b").value // Future(Left("a is not a number")) |
There was a problem hiding this comment.
If you want to show the output, we should move these comments to a "non silent tut block", that way nobody can forget to update the comments if the code is changed.
There was a problem hiding this comment.
Using tut:book, it shows
divisionProgramAsync("4", "2").value
// res4: scala.concurrent.Future[Either[String,Double]] = Future(<not completed>)
divisionProgramAsync("a", "b").value
// res5: scala.concurrent.Future[Either[String,Double]] = Future(<not completed>)which is why I opted to use the comments.
There was a problem hiding this comment.
IMO, doesn't hurt to do an Await.result here.
| val parseResult = for { | ||
| a <- eitherA | ||
| b <- eitherB | ||
| } yield (a, b) |
There was a problem hiding this comment.
You could write (eitherA, eitherB).tupled.
There was a problem hiding this comment.
If I were a new user, the applicative syntax wouldn't be well known to me, so I'll just replace the parseResult snippet with a pattern match on the eithers.
| case l@Left(err) => Future.successful(Left(err)) | ||
| (eitherA, eitherB) match { | ||
| case (Right(a), Right(b)) => divideAsync(a, b) | ||
| case (l@Left(err), _) => Future.successful(Left(err)) |
There was a problem hiding this comment.
l is unused here, right? Why not just make it Left(err)? :)
|
LGTM, I left a minor comment there but could be addressed by another PR. |
| import cats.implicits._ | ||
|
|
||
| def parseDouble(s: String): Either[String, Double] = | ||
| Try(s.toDouble).map(Right(_)).getOrElse(Left(s"$s is not a number")) |
There was a problem hiding this comment.
Super nitpicky and unreasonable comment: s could still technically be a number that can't be represented as a Double. It might be best to leave it simple and just say "$s is not a valid double.
I've been using
EitherTrecently, so I thought it would be a good idea to add some docs for it.