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

consider merging ReadOnly... APIs #256

Open
propensive opened this issue May 17, 2020 · 3 comments
Open

consider merging ReadOnly... APIs #256

propensive opened this issue May 17, 2020 · 3 comments

Comments

@propensive
Copy link
Collaborator

propensive commented May 17, 2020

The read-only APIs give us faster compilation by introducing new supertraits for the CaseClass and SealedTrait interfaces which save the cost of generating methods we are not going to use. We don't have "write-only" interfaces, but a similar approach could be used, at the expense of more complexity in the lifted interface hierarchy.

However, if Magnolia knew that one of these approaches was required, it could use the same interface, but simply implement the unused methods with ???.

There is a question about how it would determine this, but I would suggest checking whether final val readOnly = true or final val writeOnly = true exist in the derivation object; each should be typed as true.type, hence the use of final. (They could also be typed as Unit and their mere existence would be the application of that flag.)

@jatcwang
Copy link
Contributor

jatcwang commented Sep 24, 2020

Interesting idea! How about a sealed trait of ReadOnly/WriteOnly/ReadWrite? With them being case objects it'll work too right? I can take a stab at this if you want?

@joroKr21
Copy link
Contributor

That sounds better. But I'm still not sure how the API should look like. My only worry is that making it name-based could be too obscure.

@jatcwang
Copy link
Contributor

Something like this?

object ShowDerivation {
  val typeclassAccess = TypeclassAccess.ReadOnly
  type Typeclass[T] = Show[T]

  def combine[T](ctx: CaseClass[Show, T]): Show[T] = ...

  // etc
}

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

No branches or pull requests

3 participants