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

Make it easier to compose ConcreteCodecs #104

Open
Choc13 opened this issue Feb 27, 2021 · 5 comments
Open

Make it easier to compose ConcreteCodecs #104

Choc13 opened this issue Feb 27, 2021 · 5 comments

Comments

@Choc13
Copy link

Choc13 commented Feb 27, 2021

Currently jreqWith etc require that the codec passed to them is a "pair of functions" type codec. Such as (JsonValue -> a' ParseResult) * (a' -> JsonValue). However, when defining codecs for more complex types it's preferable to use the applicative style which creates a ConcreteCodec. It's then cumbersome to use this codec in another codec as it requires converting back to "pair of functions" type by doing codec |> Codec.ofConcrete |> Codec.compose jsonObjToValueCodec.

As an example consider the following types:

type Foo = { X: int }

type Bar = { Foo: Foo }

Then you might want to define codecs for them in another module which currently requires writing them like this:

module Foo =
    let codec =
        fun x -> { X = x }
        <!> jreq "x" (fun x -> x.X)

module Bar =
    let codec =
        fun foo -> { Foo = foo }
        <!> jreqWith (Foo.codec |> Codec.ofConcrete |> Codec.compose jsonObjToValueCodec) "foo" (fun x -> x.Foo)

So it would be nicer if we could avoid having to convert between the two codec representations.

@gusty
Copy link
Member

gusty commented Mar 3, 2021

I think a good solution should be to duplicate all those functions for both modules / types.

This means in your example you should be able to do directly ConcreteCodec.compose.

Maybe we should come up with a better / shorter alias for ConcreteCodec, specially considering we'll be encouraging it use over the pair of functions.

@Choc13
Copy link
Author

Choc13 commented Mar 3, 2021

Yes I think renaming ConcreteCodec might a good idea. It seems to me that ConcreteCodec is actually a codec specifically for a JObject only, whereas the more general Codec type works for any JValue which might include JObject. Is that correct?

If so then how about naming it ObjectCodec?

@gusty
Copy link
Member

gusty commented Mar 3, 2021

... whereas the more general Codec type works for any JValue which might include JObject. Is that correct?

No, actually they are both very generic, the main difference being that ConcreteCodec is a type with associated operations, whereas the tupled functions are just tupled functions so they don't carry codec operations nor intention.

@Choc13
Copy link
Author

Choc13 commented Mar 4, 2021

Hmm ok, I think my confusion came about when working with them and trying to convert between concrete and pair of functions using Codec.ofConcrete and Codec.toConcrete. As it seems like it's possible to take JsonObjCodec and call Codec.ofConcrete on it which gives you back a pair of functions between IReadOnlyDictionary and JsonValue, but it wasn't possible to say write JsonCodec.string |> Codec.toConcrete.

Also when I was trying to write jfromWith (as shown in #103) it only made sense to define this on JsonObjCodec as it doesn't make sense to "lift" a simple value into a JSON object, because it would need to be nested under some key.

@gusty
Copy link
Member

gusty commented Mar 4, 2021

Yes, we need to provide codecs like JsonCodec.string as ConcreteCodec as well, possibly under a different module.

I'll think what's the best way to accomplish this.

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

2 participants