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

add support for auto encoding of wrapped types #199

Merged
merged 5 commits into from
Feb 26, 2016

Conversation

godenji
Copy link
Contributor

@godenji godenji commented Feb 24, 2016

This PR addresses ID handling issue

case class Entity(x: Wrapped)

val q = quote {
(x: Wrapped) => query[Wrapped].filter(_.value == x)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is filter comparing an Int (_.value) to a Wrapped instance?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it is, but should not, I'll update PR, all the queries should be against Entity with wrapped param as argument.

@gustavoamigo
Copy link
Contributor

@godenji what problem does this PR solve?

@godenji
Copy link
Contributor Author

godenji commented Feb 24, 2016

@gustavoamigo automatic encoding of, for example, Id types (or any single value constructor that extends MappedValue). Basically eliminates the need to define implicit encoder/decoder boilerplate for every custom wrapped type.

Can also be used as basis for Id types like primary/foreign keys. Something like:

trait Id[T] extends Any with MappedValue[T] {
  def empty: Boolean = value == Id.zero
}
object Id{
  private val(intZero,doubleZero,longZero,stringZero,uuidZero) = (
    0, 0.0, 0L, "", new java.util.UUID(0L,0L)
  )
  /** default Id value's type */
  type Value = Int

  /** default Id */
  type Default = Id[Value]

  /** default zero value */
  val zero: Value = intZero
}

case class UserId(value: Id.Default) extends Id[Id.Default]
case class CompanyId(value: Id.Default) extends Id[Id.Default]
...

Although there's probably a better approach (i.e. not relying on static object).

@fwbrasil fwbrasil changed the title add support for auto encoding of wrapped types [WIP] add support for auto encoding of wrapped types Feb 24, 2016
@godenji
Copy link
Contributor Author

godenji commented Feb 25, 2016

@gustavoamigo @fwbrasil @jilen @lvicentesanchez can you guys take another look at this?

Maybe I didn't explain clearly enough: this PR gives us mapping of wrapped types for free

From the updated README section on encoding

import io.getquill.sources._

case class UserId(value: Int) extends AnyVal with WrappedValue[Int]
case class User(id: UserId, name: String)

val q = quote {
  (id: UserId) => for {
    u <- query[User] if u.id == id
  } yield u
}
db.run(q)(UserId(1))

// SELECT u.id, u.name FROM User u WHERE (u.id = 1)

Pretty simple implementation, just a couple of traits and an implicit conversion. Scala's a strongly typed language, let's use types to our advantage 😄

@lvicentesanchez
Copy link
Contributor

I need to look at this more carefully but as I use value classes a lot not having to write wrappers would be great. I'm going to check out the PR tomorrow and use it on my pet project.

Sent from my iPad

On 25 Feb 2016, at 23:39, godenji notifications@github.com wrote:

@gustavoamigo @fwbrasil @jilen @lvicentesanchez can you guys take another look at this?

Maybe I didn't explain clearly enough: this PR gives us mapping of wrapped types for free

From the updated README section on encoding

import io.getquill.sources._

case class UserId(value: Int) extends AnyVal with WrappedValue[Int]
case class User(id: UserId, name: String)

val q = quote {
(id: UserId) => for {
u <- query[User] if u.id == id
} yield u
}
db.run(q)(UserId(1))

// SELECT u.id, u.name FROM User u WHERE (u.id = 1)
Pretty simple implementation, just a couple of traits and an implicit conversion. Scala's a strongly typed language, let's use types to our advantage


Reply to this email directly or view it on GitHub.

@godenji
Copy link
Contributor Author

godenji commented Feb 26, 2016

but as I use value classes a lot

@lvicentesanchez exactly, got 50+ tables in a production app and definitely don't want to write implicit encoders for UserId, OrderId, etc. when I switch the backend over to Quill.

@fwbrasil fwbrasil changed the title [WIP] add support for auto encoding of wrapped types add support for auto encoding of wrapped types Feb 26, 2016
@fwbrasil
Copy link
Collaborator

👍

Approved with PullApprove

@fwbrasil
Copy link
Collaborator

@lvicentesanchez Do you mind if we merge this change and you test it on master?

@jilen
Copy link
Collaborator

jilen commented Feb 26, 2016

👍

Approved with PullApprove

@lvicentesanchez
Copy link
Contributor

Yes, I don't mind that :)
👍

Approved with PullApprove

@godenji
Copy link
Contributor Author

godenji commented Feb 26, 2016

@fwbrasil @lvicentesanchez @gustavoamigo @jilen looks like updating the branch requires committers to re-approve the PR? That or there's some kind of pullapprove cache that isn't getting cleared.

Otherwise, good to go for merging.

@lvicentesanchez
Copy link
Contributor

👍
On 26 Feb 2016 14:24, "godenji" notifications@github.com wrote:

@fwbrasil https://github.com/fwbrasil @lvicentesanchez
https://github.com/lvicentesanchez @gustavoamigo
https://github.com/gustavoamigo @jilen https://github.com/jilen looks
like updating the branch requires committers to re-approve the PR? That or
there's some kind of pullapprove cache that isn't getting cleared.

Otherwise, good to go for merging.


Reply to this email directly or view it on GitHub
#199 (comment).

godenji added a commit that referenced this pull request Feb 26, 2016
add support for auto encoding of wrapped types
@godenji godenji merged commit d4abe34 into zio:master Feb 26, 2016
@fwbrasil fwbrasil mentioned this pull request Mar 1, 2016
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

Successfully merging this pull request may close these issues.

5 participants