Add transform and subflatMap to OptionT and XorT

[liff]

As per gitter discussion; proposing to add transform and subflatMap methods to OptionT and XorT. I suppose they should be in StreamingT as well but it's not currently obvious to me how to go about the implementation.

TODO:

• agree on the name :)
• add tests
• implement for StreamingT?

[ceedubs]

Thanks, @liff!

I think you can relax all of the Monad constraints to Functor.

[codecov-io]

Current coverage is 76.13%

Merging #571 into master will increase coverage by +0.09% as of 86b71fc

@@            master    #571   diff @@
======================================
  Files          159     159       
  Stmts         2066    2070     +4
  Branches        69      69       
  Methods          0       0       
======================================
+ Hit           1571    1576     +5
  Partial          0       0       
+ Missed         495     494     -1

Review entire Coverage Diff as of 86b71fc

Powered by Codecov. Updated on successful CI builds.

[non]

I'm fine with the names. I agree with @ceedubs that you can relax the constraints. I also think it might be nice to write value.map(f) instead of F.map(value)(f) but that is somewhat subjective.

I'd say you should proceed with the tests and then we can think about how (or if) to make this work for StreamingT 😉

[aryairani]

transform vs subMap? Or is there history behind the names?

[liff]

Relaxed constraints.

@non, wrt. value.map(f), that would require importing cats.syntax.functor and it doesn't seem to be consistent with the other methods around?

@refried, the only history is in the linked gitter conversation :)

[ceedubs]

This looks good to me. I think it's good to merge once it gets some tests.

I think the lack of consistency of where we are using syntax and where we aren't is a bigger issue that we can tackle separately. It's made a bit trickier by the fact that Machinist and Simulacrum currently don't play together very nicely. Hopefully typeclassic saves the day! :)

[non]

Sure, let's leave my syntax suggestion out of this.

[liff]

Sorry guys, somewhat busy at the moment; going to get back to this later this week.

[ceedubs]

@liff no worries; no rush.

[liff]

Some tests added. Let me know if that looks sufficient.

Implementation for StreamingT still eludes me, unfortunately. It's quite different from these two specimen.

I also feel obliged to add @TomasMikula's comments about Hoist here, for the record.

[TomasMikula]

On that note, both OptionT and XorT can be made Hoists (reference from scalaz: MaybeTHoist, EitherTHoist) and then subflatMap can be implemented in terms of hoist.

[TomasMikula]

And then so can be StreamingT.

[TomasMikula]

It's still open (at least to me) whether there are interesting monad transformers that are not Hoists but have a meaningful subflatMap, but these three are not such.

[ceedubs]

👍

I could see Hoist being handy for this sort of thing in the future, but I think these methods are also useful as they are in this PR.

[ceedubs]

Sorry for the hassle @liff, but could you please resolve the merge conflict?

[liff]

Done.

[ceedubs]

👍 thanks @liff!

[non]

Looks good to me. 👍