Skip to content
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

Reduce allocation overhead by removing Kleisli layer #108

Open
rlecomte opened this issue May 21, 2018 · 2 comments
Open

Reduce allocation overhead by removing Kleisli layer #108

rlecomte opened this issue May 21, 2018 · 2 comments
Labels
enhancement New feature or request help wanted Extra attention is needed

Comments

@rlecomte
Copy link

Hammock Interpreter have HttpClient as constructor parameter. So why use Kleisli[F, HttpClient, ?] in the internal machinery?

We could just use Async effect without any other layer.

@pepegar
Copy link
Owner

pepegar commented May 23, 2018

Hi @rlecomte!

Yeah, you're right that receiving it as a constructor param and using
it in the transK method is a bit repetitive.

However, since the interface we need to follow is the one from
InterpTrans,
we need it in both places, as for now, since the transK method is
being used in other projects.

I've however thought of changing the InterpTrans more than
once, that meaning that since for all implementations of the
interpreter we'd need a value of the target's http client (apache's
HttpClient, Akka's HttpExt...), I'd delete the trans
method from the typeclass and only use the transK method.

trait InterpTrans[F[_], Client] {
  def transK(implicit S: Sync[F]): HttpF ~> Kleisli[F, Client, ?]
}

And we'd create instances of the like this:

implicit object AkkaInterpreter extends InterpTrans[F, HttpExt] {
  def transK...
}

What do you think about it? if we end up creating interpreters like
so, we'd end up not needing the constructor parameter.

@rlecomte
Copy link
Author

rlecomte commented May 24, 2018

I'm not sure to understand the whole picture but here is my 2 cents :

/*
  Keep interpreter signature as simple as possible. Just a F type without any behavior.
 */
trait InterpTrans[F[_]] {
  def trans: HttpF ~> F
}
/*
 Just an alias for Kleisli interpreter
*/
trait KleisliInterTrans[F[_], Client] extends InterpTrans[Kleisli[F, Client, ?]]
/*
 Here is the trick : deal with legacy by using client in context to execute kleisli interpreter
*/
trait LegacyTrans[F, Client] extends InterpTrans[F[_]] {
  val client: Client
  val kleisliTrans: KleisliInterTrans[F, Client, ?]

  def trans: HttpF ~> F = kleisliTrans.transK andThen λ[Kleisli[F, Client, ?] ~> F](_.run(client))

  def transK: HttpF ~> Kleisli[F, Client, ?] = kleisliTrans.trans
}
/*
An implementation for apache http client
*/
case class ApacheHttpClientTrans[F[_]](client: HttpClient) extends LegacyTrans[F, HttpClient] {
  val kleisliTrans = ApacheHttpClientKleisliTrans
}

object ApacheHttpClientKleisliTrans extends KleisliInterTrans[F, HttpClient] {
   //implementation
}

In this way, we don't introduce any breaking change and keep InterpTrans as simple as possible. This allow to implement any interpreter we want.

@pepegar pepegar added enhancement New feature or request help wanted Extra attention is needed labels Jun 26, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

2 participants