ProjectSeptemberInc / freek

Freek, a freaky simple Free to combine your DSL seamlessly
Other
198 stars 19 forks source link

Adding freekF method #3

Closed zainab-ali closed 8 years ago

zainab-ali commented 8 years ago

Adding freekF, a method which lifts a Free[F, A]

mandubian commented 8 years ago

Thanks a lot @zainab-ali for this 1st PR 👍

Could you check but I think existing function .expand does the same as yours:

https://github.com/ProjectSeptemberInc/freek/blob/master/src/main/scala/Freek.scala#L13-L19

and

https://github.com/ProjectSeptemberInc/freek/blob/master/src/main/scala/package.scala#L30

dwhitney commented 8 years ago

perhaps freekF is a better name for expand? Hmm.... expand is more explanitory, but freekF is maybe a little more familiar?

On Thu, Jun 16, 2016 at 7:40 AM, Pascal Voitot notifications@github.com wrote:

Thanks a lot @zainab-ali https://github.com/zainab-ali for this 1st PR 👍

Could you check but I think existing function .expand does the same as yours:

https://github.com/ProjectSeptemberInc/freek/blob/master/src/main/scala/Freek.scala#L13-L19

and

https://github.com/ProjectSeptemberInc/freek/blob/master/src/main/scala/package.scala#L30

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/ProjectSeptemberInc/freek/pull/3#issuecomment-226461255, or mute the thread https://github.com/notifications/unsubscribe/AACCoSZjCPWJvFGbapIPwihDGXASOK-Wks5qMTYvgaJpZM4I2Y7m .

mandubian commented 8 years ago

The meaning is really to expand your Free[DSL, A] into a bigger DSL, Free[BigDSL, A] freekF wouldn't bear this meaning from my point of view but I'm not so good at finding names as you know :D WDYT?

dwhitney commented 8 years ago

yeah I agree with @mandubian

On Thu, Jun 16, 2016 at 9:49 AM, Pascal Voitot notifications@github.com wrote:

The meaning is really to expand your Free[DSL, A] into a bigger DSL, Free[BigDSL, A] freekF wouldn't bear this meaning from my point of view but I'm not so good at finding names as you know :D WDYT?

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/ProjectSeptemberInc/freek/pull/3#issuecomment-226490260, or mute the thread https://github.com/notifications/unsubscribe/AACCoVMxZ9qJrzIpT3eiHS5glg9pyFmFks5qMVRYgaJpZM4I2Y7m .

zainab-ali commented 8 years ago

There's a subtle difference between expand and freekF.

expand takes a Free[DSL, A] where DSL is a subtype of CoproductK and transforms it into a bigger dsl Free[BigDSL, A]

freekF takes a Free[F, A], where F has no constraints, and transforms it into a Free[DSL, A], and then transforms it into a Free[BigDSL, A]

I'm all for having a single method which tackles both cases, though I'm not sure how this could be done.

As for the name, I prefer expand, but didn't want naming conflicts. EDIT: I can rename the method to expand without conflicts. What do you think?

dwhitney commented 8 years ago

ok, I see the difference, and yeah that seems like a better version of expand

mandubian commented 8 years ago

oh I see, you're right! maybe we could create a new custom SubCop that works in both cases (F not CoproductK or CoproductK) to have one single function expand

zainab-ali commented 8 years ago

We could remove the CoproductK contstraint on L in SubCop[L[_] <: CoproductK[_], L2[_] <: CoproductK[_]], then write two implicits: one for a subtype of CoproductK, and one for something which isn't a subtype of CoproductK.

But I think that would complicate things far more than having two expand methods.
Do you have an idea of what the custom SubCop would look like?

mandubian commented 8 years ago

I'll try to write it to see if it's reasonable or not... I'm not against having several functions if it lessens the risk of crazy compile errors

zainab-ali commented 8 years ago

Any updates on this one? If not, I'll resolve the conflicts. IMO the current implementation is simpler than implicit manipulation.

mandubian commented 8 years ago

Could you try to synchronize with current master? Things have moved a bit since this PR. I have changed anything to SubCop yet as I was working on other ideas. Thanks

zainab-ali commented 8 years ago

Synchronised

zainab-ali commented 8 years ago

Re-synchronised

mandubian commented 8 years ago

Thanks dude! I'm going to take that into account asap... busy lately ;)