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

Remove *R methods #25

Closed
gaelrenoux-datadome opened this issue Mar 31, 2021 · 4 comments
Closed

Remove *R methods #25

gaelrenoux-datadome opened this issue Mar 31, 2021 · 4 comments

Comments

@gaelrenoux-datadome
Copy link
Contributor

Hey, sorry to bump this, but I wanted to share something we use internally:

  def transaction[E, A](tx: ZIO[Connection, E, A]): ZIO[Transactor, E, A] =
    Database.transactionOrDie(tx)

  def transaction[R, E, A](tx: ZIO[Connection with R, E, A])(implicit ev: R <:< Has[_]): ZIO[Transactor with R, E, A] =
    Database.transactionOrDieR(tx)

A very thin wrapper, delegating to tranzactio. I was able to create two overloads called transaction. Normally this doesn't work, since [E, A] and [R, E, A] have the same erasure, but turns out that sticking an implicit causes them to be different as far as Scala is concerned!

So I can have my cake and eat it too - a single transaction function (from the user's perspective) that handles either Connection or Connection with R.

Not sure if that ship had sailed (now that you've published v2.0), but maybe you could use this trick and remove the R overloads completely! :)

Originally posted by @hmemcpy in #24 (comment)

@hmemcpy
Copy link
Contributor

hmemcpy commented Mar 31, 2021

Awesome! Thanks so much. Turns out this was an incomplete copy-paste, since the R also need to have a <: Has[_] constraint. So together with the implicit parameter this should be enough for Scala to treat those as distinct functions!

One more thing that happened, and I'm not sure why that is, was that calling Database.transactionOrDieR(tx) didn't work, and I got the strangest compilation error:

Transactor.scala:55:32: type mismatch;
 found   : zio.ZIO[io.github.gaelrenoux.tranzactio.doobie.Connection with R(in method transaction),E,A]
    (which expands to)  zio.ZIO[zio.Has[doobie.util.transactor.Transactor[zio.Task]] with R(in method transaction),E,A]
 required: zio.ZIO[io.github.gaelrenoux.tranzactio.doobie.Connection with R(in method transactionOrDieR),E,A]
    (which expands to)  zio.ZIO[zio.Has[doobie.util.transactor.Transactor[zio.Task]] with R(in method transactionOrDieR),E,A] [55:32]

(notice the R(in method transaction) and R(in method transactionOrDieR)) - for some reason it didn't know it was the same R! I was able to work around this by explicitly annotating it with: Database.transactionOrDieR[R, E, A](tx)
Intellij paints it red, for some reason, but at least it compiles... 🤷‍♂️

@gaelrenoux
Copy link
Owner

OK, I finally took some time to play around this a bit, and it doesn't work (not easily, at least). The issue is, the transaction method have optional parameters (the commitOnFailure flag, and the ErrorStrategies implicit), so I hit the error: multiple overloaded alternatives of method XXX define default arguments.
Not sure what can be done. Having separate methods instead of optional arguments would complicate things a lot.

@hmemcpy
Copy link
Contributor

hmemcpy commented Jun 12, 2021

Yeah, unfortunately I had the same conclusion. Scala really doesn't like default arguments on overloaded methods. So without redesigning the API there's no easy solution. Oh well, it was a nice hack anyway :)

@gaelrenoux
Copy link
Owner

Closing for now, might come back later if we completely redo the API at some point.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants