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

Replace schematic implementation by schema visitor #296

Merged
merged 29 commits into from
Jul 11, 2022

Conversation

daddykotex
Copy link
Contributor

No description provided.

@daddykotex daddykotex changed the title Document[Decoder|Encoder]SchemaVisitor Replace schematic implementation by schema visitor Jul 8, 2022
): DocumentEncoder[Map[K, V]] = {
val keyEncoder = apply(key)
val valueEncoder = apply(value)
fromNotKey[Map[K, V]] { map =>
Copy link
Contributor

Choose a reason for hiding this comment

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

the fromNotKey should disappear. Instead, we should have another SchemaVisitor that would compute a Option[KeyEncoder[A]] (and same on the Decoder side), the interface of which would be something like :

trait KeyEncoder[A] {
     def apply(a) : String 
} 

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Do you mind if I do these updates in a subsequent PR? There are a few things that could be re-written. I saw some of them but I told myself: let's do the mechanical translation first (remove all instances of schematic) and then we'll improve the visitors.

it will reduce the review overhead. you can consider this PR as a one-to-one translation (mostly, because some signature are different and so some code has to change)

@daddykotex
Copy link
Contributor Author

Last push removes Schematic altogether, but it's based on #295 so I'll make this as Draft until the other one is merged back in here

@daddykotex daddykotex marked this pull request as draft July 8, 2022 20:19
@Baccata Baccata self-requested a review July 10, 2022 11:27
@daddykotex daddykotex marked this pull request as ready for review July 11, 2022 12:31
Baccata and others added 2 commits July 11, 2022 23:47
... to make it easier to write memoised dispatching logic

The `union` method in SchemaVisitor is an obvious footgun, because it makes it a hard to memoise the compilation of schemas for each alternative. 

This PR revises the `union` method to expose a new construct `Alt.Dispatcher`, which does the heavylifting of memoizing the result of compiling schemas, and dispatching the union instance to the correct alternative. 

* Creates a `Alt.Dispatcher` construct that aims at streamlining the
implementation of the `union` methods in SchemaVisitors, which is the
hardest due to the fact that the implementor has to think about memoization.
* Creates an `EncoderK` typeclass (used by Alt.Dispatcher) to generalise
over what is an encoder, and streamline further the implementation of
union encoders.
* Examplifies the use of `Alt.Dispatcher` in DocumentEncoderSchemaVisitor

This should remove the need for #260
@Baccata Baccata merged commit f110d22 into main Jul 11, 2022
@Baccata Baccata deleted the dfrancoeur/kill-schematic branch July 11, 2022 22:02
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

3 participants