-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
add OptionT.liftF and update related docs and tests. Close issue #659 #667
Changes from 2 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -51,6 +51,33 @@ val noWelcome: OptionT[Future, String] = customGreetingT.filterNot(_.contains("w | |
val withFallback: Future[String] = customGreetingT.getOrElse("hello, there!") | ||
``` | ||
|
||
## From `Option[A]` and/or `F[A]` to `OptionT[F, A]` | ||
|
||
Sometimes you may have an `Option[A]` and/or `F[A]` and want to *lift* them into an `OptionT[F, A]`. For this purpose `OptionT` exposes two useful methods, namely `fromOption` and `liftF`, respectively. E.g.: | ||
|
||
```tut:silent | ||
import scala.concurrent.Future | ||
import scala.concurrent.ExecutionContext.Implicits.global | ||
|
||
import cats.data.OptionT | ||
import cats.std.future._ | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. These imports already happen earlier in the file. Do we want them again here? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, I noticed but I wanted to example to be self-contained. By the way, if you don't import scala> OptionT.liftF(f)
<console>:17: error: could not find implicit value for parameter F: cats.Functor[scala.concurrent.Future]
OptionT.liftF(f) Do you think I should file this in a different issue or it's not a problem? I mean, even if you import Anyway if you think it's not a problem I can remove those imports. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I can see the benefit in self-contained examples. My only concern is that currently with tut I don't think we have a way to clear previous imports in the same file. So we run the risk of duplicating imports without actually guaranteeing that the example is self-contained. I don't have any really strong feelings on this though, so whatever you think is best. Regarding the misleading missing implicit value message: There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I removed those imports and filed the #670 for that issue. |
||
|
||
val greetingFO: Future[Option[String]] = Future.successful(Some("Hello")) | ||
|
||
val firstnameF: Future[String] = Future.successful("Jane") | ||
|
||
val lastnameO: Option[String] = Some("Doe") | ||
|
||
val ot: OptionT[Future, String] = for { | ||
g <- OptionT(greetingFO) | ||
f <- OptionT.liftF(firstnameF) | ||
l <- OptionT.fromOption(lastnameO) | ||
} yield s"$g $f $l" | ||
|
||
val result: Future[Option[String]] = ot.value // Future(Some("Hello Jane Doe")) | ||
|
||
``` | ||
|
||
## Beyond map | ||
|
||
Sometimes the operation you want to perform on an `Option[Future[String]]` might not be as simple as just wrapping the `Option` method in a `Future.map` call. For example, what if we want to greet the customer with their custom greeting if it exists but otherwise fall back to a default `Future[String]` greeting? Without `OptionT`, this implementation might look like: | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This may not really be something to worry about, but I'm thinking that maybe this should use
Some(_)
instead ofOption(_)
. TheOption.apply
method has logic to turnnull
intoNone
. In Cats we are generally assuming that people aren't passing aroundnull
. But if for some reason they really want to lift anull
value into theOptionT
structure, I don't think we should prevent them from doing that here.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I used
Option(_)
to preventnull
and gettingNone
instead ofSome(null)
in that case, but if you confirm we should not prevent them from doing that I'll change it toSome(_)
.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think that it should be consistent with our
Applicative[Option].pure
implementation, which usesSome
: /~https://github.com/non/cats/blob/master/core/src/main/scala/cats/std/option.scala#L14. So yeah, I think we should change this here. I don't think we should conflate the issues of lifting and de-nulling.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I totally agree! I'll change it to
Some(_)
. Thank you.