Split JsonFormat into Encoder and Decoder (or Writer and Reader)
Closed this issue · 2 comments
Currently diffson requires combined JsonFormat
for library integration, which is not be supported by some libraries such as circe by design. Here's the corresponding section in it's design document:
The public API should not contain unnecessary types. Argonaut (like many other great serialization libraries) includes a Codec type class that combines Encoder and Decoder, but it's only really useful for definitions: it allows you to define an encoder-decoder pair together, but its generally not useful as a type constraint, since many types with both Encoder and Decoder instances will not have a Codec instance. I personally find this confusing, and would prefer to keep convenience-oriented types like this out of the API.
What this means concretely: There's unlikely to be a Codec in circe until we come up with a clean way to make it useful as a type constraint (although if demand gets loud enough I would be willing to consider compromise here).
Although it's possible to create a proxy class that combines circe's Encoder
and Decoder
, I'd suggest that we separate them in diffson for more flexibility.
Hi,
I am not convinced with the arguments in your citation in general. For me it really depends on the use cases. I will review the diffson usage of Format to see if diffson always needs both encoder and decoder. If so, I don’t see it really likely for diffson to adopt this approach. Diffson is not a json library but a concrete usage of such libraries.
I do not see either what kind of flexibility it would bring in our use case. Can you please explain in more details what you mean with "more flexibility"?
It just means adding circe support as it is, without introducing unnecessary proxy layer. I have CirceDiffsonInstance
which defines a JsonFormat
like this:
trait JsonFormat[A] {
def decode(c: HCursor): Result[A]
def encode(a: A): Json
}
object JsonFormat {
implicit def apply[A](implicit encoder: Lazy[Encoder[A]], decoder: Lazy[Decoder[A]]): JsonFormat[A] =
new JsonFormat[A] {
override def decode(c: HCursor): Result[A] = decoder.value.apply(c)
override def encode(a: A): Json = encoder.value.apply(a)
}
}
Although it just works, I feel a bit uncomfortable about this because it's against circe's philosophy and it adds runtime overhead, even though it's negligible.
Looking at JsonSupport#JsonProvider
, type Marshalling[T]
is used in three places; marshal
, unmarshal
, and patchMarshaller
. IMHO it's natural to split Marshalling
into Marshall
and Unmarshall
to correspond to each function and split patchMarshaller
into patchMarshaller
and patchUnmarshaller
. Thanks to diffson's typeclass oriented interface, it'll introduce source level breakage of zero or few lines to end users.
Being more specific, JsonDiff#diff only requires Marshall
and JsonPatch#apply requires the both. For the latter, just changing the type parameter from [T: Marshalling]
to [T : Marshall : Unmarshall]
is enough.
If a JSON library provides its reader/writes only in the combined form, Marshall
and Unmarshall
can just point the same type so that there's no problem. And there's no one taking this approach AFAIK and every typeclass based JSON library already exposes separate traits regarding each case.