Add foldMap/compile FunctionK to Free companion

[johnynek]

This came up for me in Free usage. We actually didn't want to call foldMap but to build another FunctionK.

Note, an interesting corollary to noting that Free converts a FunctionK[S, M] => FunctionK[Free[S, ?], M] and noting that we always have via liftF a FunctionK[M, Free[M, ?]] then if you have some code that requires complex recursive flatmaps, you can always first convert everything to Free via liftF, then in the end use the Monad.tailRecM along with foldMap(FunctionK.id). So, in it seems that many classes of recursion, as long as you don't change the Monad, are covered by implementing tailRecM.

[codecov-io]

Current coverage is 92.01% (diff: 90.00%)

Merging #1411 into master will increase coverage by 0.33%

@@             master      #1411   diff @@
==========================================
  Files           240        242     +2   
  Lines          3608       3820   +212   
  Methods        3540       3751   +211   
  Messages          0          0          
  Branches         67         69     +2   
==========================================
+ Hits           3308       3515   +207   
- Misses          300        305     +5   
  Partials          0          0          

Powered by Codecov. Last update 86491ba...7798939

[travisbrown]

👍, but the names seem potentially a little confusing given that they reverse the usual order of arguments. I'm not sure that something like compileWith or foldMapWith is any better, though.

[non]

This seems basically good to me. Two questions though:

1. Is there a reason to define Free.compile but FreeT.interpret? These seem like they could be the same name (and either name seems OK to me).
2. Is there a reason to use FunctionK[F, G] in Free, but S ~> T in FreeT? This makes the methods seem more different than they really are.

[johnynek]

@non yeah, when adding these I noticed we were somewhat inconsistent in Free and FreeT.

I made FreeT use the standard in the FreeT file. Should I add a duplicate name or rename interpret to compile in FreeT? If we want to make such a change, sooner is better, right?

[non]

Yeah for now let's add a duplicate name, deprecate the non-standard one, and call it a day.

[non]

👍 once things are green

[non]

FWIW -- I'm fine with merging this despite not having a test to cover the deprecated method.

[travisbrown]

@non Yeah, we could define the new one in terms of the old, but then that's just another thing to do when we remove it. Having it uncovered is also a good motivation to get rid of it as soon as possible, too. :)

[johnynek]

Call me reckless but a deprecated method that just redirects to another is
not my top anxiety about code coverage.
On Tue, Oct 25, 2016 at 18:50 Travis Brown notifications@github.com wrote:

@non https://github.com/non Yeah, we could define the new one in terms
of the old, but then that's just another thing to do when we remove it.
Having it uncovered is also a good motivation to get rid of it as soon as
possible, too. :)

—
You are receiving this because you modified the open/close state.

Reply to this email directly, view it on GitHub
#1411 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/AAEJdiLTygGnMkdKaTbEyt1xWSXWEJwhks5q3twYgaJpZM4KaVdE
.