Add a map method directly onto Resource
#437
Conversation
This enables it to be found by IDEs which have issues with partial unification and feels like it should be there since the class already provides `flatMap`.
| * see `map`. | ||
| */ | ||
| def map[B](f: A => B)(implicit F: Monad[F]): Resource[F, B] = | ||
| flatMap(a => Resource.applyCase(F.pure((f(a), _ => F.unit)))) |
There was a problem hiding this comment.
I think you can just use Resource.pure instead of the manual apply case (or conversely just build the tree using Bind and Allocate). In any case the constraint on Fcan probably be weakened to Applicative
There was a problem hiding this comment.
Good catch, I got slightly hung up on following how map was implemented via Monad and ended up using applyCase, I've simplified it to using pure and weakened the F to Applicative
|
Speaking of this, somebody should also start a ticket with JetBrains for IntelliJ IDEA, maybe somebody ends up fixing their support. With precisely this example, It's been worsening lately, not sure what they are doing, probably rebuilding their engine or something. |
| @@ -87,6 +87,13 @@ class ResourceTests extends BaseTestsSuite { | |||
| Resource.liftF(fa).use(IO.pure) <-> fa | |||
| } | |||
| } | |||
| testAsync("map should be the same as going via Monad") { implicit ec => | |||
There was a problem hiding this comment.
Instead of adding this test, a better idea would be to override map in the provided Monad instance.
Codecov Report
@@ Coverage Diff @@
## master #437 +/- ##
==========================================
+ Coverage 85.44% 89.47% +4.03%
==========================================
Files 70 70
Lines 2102 2005 -97
Branches 202 209 +7
==========================================
- Hits 1796 1794 -2
+ Misses 306 211 -95 |
| * available via the `cats.Monad` instance for `Resource[F, ?]` | ||
| * | ||
| * This allows IDEs which have issues with Partial Unification to | ||
| * see `map`. |
There was a problem hiding this comment.
The reason for why this map is provided shouldn't be exposed in ScalaDoc, especially since now this implementation becomes the "source of truth" for Resource.
Just do something generic like ...
Given a mapping function, transforms the source.
This is the standard `Functor.map`.
There was a problem hiding this comment.
But I think we need another ScalaDoc comment, on Resource class, mentioning:
-Ypartial-unificationimport cats.implicits._
... to unlock all available operations.
This enables it to be found by IDEs which have issues with partial
unification and feels like it should be there since the class already
provides
flatMap.