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

Codec #1151

Merged
merged 11 commits into from
Jun 12, 2019
Merged

Codec #1151

merged 11 commits into from
Jun 12, 2019

Conversation

travisbrown
Copy link
Member

@travisbrown travisbrown commented Jun 10, 2019

I've resisted adding this thing for years, but I've finally been convinced that having Codec around for more convenient definitions (and faster semi-automatic derivation) is a good idea. This PR builds on #1150 (which is motivated by this change but should stand on its own) and includes @OlivierBlanvillain's change from #301. It doesn't build on @lorandszakacs's #811 at the moment, but I'll probably pull some tests from there.

Note that the intention is that Codec should only be used to make defining instances more convenient, and not as a constraint. The core module provides no Codec instances for any standard library types, and there is no machinery for summoning Codec instances given an encoder-decoder pair. Fully-automatic generic derivation does not (and probably will not) provide Codec instances, although semi-automatic derivation is supported in both the vanilla circe-generic form and with configuration in circe-generic-extras.

The motivation is to speed up semi-automatic generic derivation (it should be not too far off twice as fast as deriving separate encoders and decoders, but I've not actually done any benchmarking yet) and to avoid forcing users to write out member names twice when using forProductN.

Update: I put together a file with 200 case classes and either deriveCodec or a deriveDecoder / deriveEncoder pair in the companion class, and after half a dozen runs of each on Scala 2.12.8, the new deriveCodec approach consistently compiles in 20-21 seconds, while the old approach takes 35-36 seconds. So about 42% faster for that example case, which is actually a little better than I expected.

The implementation currently supports writing this:

import io.circe.Codec
import io.circe.generic.semiauto.deriveCodec

case class Foo(a: String, b: List[Boolean], c: Double)

object Foo {
  implicit val fooCodec: Codec[Foo] = deriveCodec
}

…or this:

import io.circe.Codec

case class Foo(a: String, b: List[Boolean], c: Double)

object Foo {
  implicit val fooCodec: Codec[Foo] =
    Codec.forProduct3("a", "b", "c")(Foo(_, _, _))(foo => (foo.a, foo.b, foo.c))
}

…instead of this:

import io.circe.{ Decoder, ObjectEncoder }
import io.circe.generic.semiauto._

case class Foo(a: String, b: List[Boolean], c: Double)

object Foo {
  implicit val decodeFoo: Decoder[Foo] = deriveDecoder
  implicit val encodeFoo: ObjectEncoder[Foo] = deriveEncoder
}

…or this:

import io.circe.{ Decoder, Encoder, ObjectEncoder }

case class Foo(a: String, b: List[Boolean], c: Double)

object Foo {
  implicit val decodeFoo: Decoder[Foo] =
    Decoder.forProduct3("a", "b", "c")(Foo(_, _, _))

  implicit val encodeFoo: ObjectEncoder[Foo] =
    Encoder.forProduct3("a", "b", "c")(foo => (foo.a, foo.b, foo.c))
}

It still needs tests and better docs and probably some clean-up.

@codecov-io
Copy link

codecov-io commented Jun 10, 2019

Codecov Report

Merging #1151 into master will decrease coverage by 0.06%.
The diff coverage is 68.62%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1151      +/-   ##
==========================================
- Coverage   86.39%   86.32%   -0.07%     
==========================================
  Files          83       88       +5     
  Lines        2837     2962     +125     
  Branches      119      122       +3     
==========================================
+ Hits         2451     2557     +106     
- Misses        386      405      +19
Impacted Files Coverage Δ
.../generic/extras/encoding/ReprAsObjectEncoder.scala 50% <ø> (ø) ⬆️
.../core/shared/src/main/scala/io/circe/Decoder.scala 86.21% <ø> (-0.51%) ⬇️
...ala/io/circe/generic/codec/ReprAsObjectCodec.scala 0% <0%> (ø)
...circe/generic/extras/codec/ReprAsObjectCodec.scala 0% <0%> (ø)
...es/core/shared/src/main/scala/io/circe/Codec.scala 0% <0%> (ø)
.../io/circe/generic/codec/DerivedAsObjectCodec.scala 100% <100%> (ø)
...io/circe/generic/util/macros/JsonCodecMacros.scala 91.66% <100%> (-3.34%) ⬇️
...ared/src/main/scala/io/circe/generic/Deriver.scala 100% <100%> (ø) ⬆️
...red/src/main/scala/io/circe/generic/semiauto.scala 100% <100%> (ø) ⬆️
.../io/circe/generic/extras/ConfigurableDeriver.scala 100% <100%> (ø) ⬆️
... and 12 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3ef0293...c8e6e9d. Read the comment docs.

@tpolecat
Copy link

doobie has Meta that works exactly the same way. You can use it to introduce a symmetric Get/Put pair but you never demand it as a evidence. It's bad and I feel bad but users like it.

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.

None yet

4 participants