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

Support default values for case class members #12

Closed
travisbrown opened this issue May 30, 2017 · 6 comments
Closed

Support default values for case class members #12

travisbrown opened this issue May 30, 2017 · 6 comments

Comments

@travisbrown
Copy link
Member

There are (at least) two questions about how this should work:

  1. Should it always be on, or optional?
  2. Should it be possible for defaults to be omitted during encoding? If yes, should that be optional or just the way things are? Also if yes, should it rely on universal equality or cats.kernel.Eq?
@stephennancekivell
Copy link

As a user I dont think this should be on by default. I dont think its a feature most dev's expect and can lead to surprises.

@ariskk
Copy link

ariskk commented Oct 7, 2017

I came here to open an issue about exactly that.
Re 1: I would have it optional, pretty much the same as Configuration.default.withDefaults works.
Re 2: Example from personal experience. I have seen defaults being used as a poor man's schema evolution mechanism in various systems, eg Kafka or Elasticsearch. If the model assumes a default value and this is omitted during encoding, each consumer needs to make its own assumptions about it. This becomes particularly tricky at the edge of a system, namely in interactions with clients.
Eg
User(id: Id[User], verified: Boolean = false) The client needs to know the default value if this is omitted during encoding. I am sure there arguments in favour of the opposite, thus I would for optional and by default on.
It would be great to have this feature. The derived codecs in generic.extras tend to be very very very slow to compile.

@scalway
Copy link

scalway commented Jan 5, 2018

For me it's important because upickle does that. Upickle is deprecated and right now I'm looking for replacement.
Re 2: I have use it as schema evolution mechanism, but I've share schema between client/server/other tools. Still I thing is good to keep it off by default (mostly due to compatibility issue).

@travisbrown
Copy link
Member Author

This was done in #91 and #100 and is the default behavior.

@shrynx
Copy link
Contributor

shrynx commented Mar 14, 2020

Curious what's the rationale for defaulting to true for useDefaults ?
This behavior is different from circe generic and semi auto default behaviours

making it a breaking change from being a drop-in replacement for generic/semi-auto (and also < 12)

@travisbrown
Copy link
Member Author

@shrynx To be honest my own work on circe-derivation has mostly been focused on supporting the needs of my current employer (originally Stripe, now Permutive). This is why there are only milestone releases and zero binary compatibility guarantees, etc. Matching the behavior of circe-generic and circe-generic-extras as closely as possible is one of the goals of the project, but for me at least a secondary one.

I think we started using default values by default because that's what what @aparo did in #91, it worked for me, and nobody complained at the time. We can definitely revisit that decision, so please feel free to open a new issue or PR!

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

5 participants