Skip to content
This repository has been archived by the owner on Oct 24, 2023. It is now read-only.

add post about modelling inheritance #39

Merged
merged 3 commits into from
Sep 13, 2023
Merged

Conversation

gregsdennis
Copy link
Member

I came up with this idea when starting work on a codegen library. It's adjacent to something I'm using in that implementation to work out whether a schema is supported.

@Relequestual
Copy link
Member

A few comments...

Do you know about Device Tree used in Linux?
https://www.devicetree.org
https://github.com/devicetree-org/dt-schema

It occurs to me that it could be possible to prevent schemas which are abstract classes from being used directly...
If you used a dynamic reference in the abstract class schema, which referenced a definition of isNotAbstractClass or something, the schema value could be false. Doing so means that the abstract class schema always fails validation... until a derrived schema defines a dynamicAnchor of the same name with a schema value of true or empty object.
It would add another step, but it could help prevent mistakes. But then, simply naming the schema "Abstract..." could be enough to help prevent the same mistakes.

Do you think it's worth mentioning that if you want to have schemas built at build time, you could do that sort of thing using Jsonnet? Details would be out of scope of the article, but people could dig if they wanted to.

@Relequestual
Copy link
Member

It looks like Device Tree are using their own modified version of draft-07, where the meta-schema is defined in 2019-09... so I'm not sure what exactly is going on 😅
https://github.com/devicetree-org/dt-schema/blob/main/dtschema/meta-schemas/base.yaml

@gregsdennis
Copy link
Member Author

I looked at using dynamic references, but couldn't get it all working right. What you suggest is similar to what I used to describe generic types.

I think leaving them out is simpler anyway. It might be better to start simple and maybe have a follow-up post for advanced cases.

@gregsdennis
Copy link
Member Author

Do you know about Device Tree used in Linux?

I had not heard of this. Devices just presented more of a "property" inheritance opportunity, whereas most polymorphism examples use a "function" inheritance, e.g. Animal defines a .MakeSound() function that both Cat and Dog implement differently. But in JSON Schema, we don't care about functions; we care about properties.

Copy link
Member

@jdesrosiers jdesrosiers left a comment

Choose a reason for hiding this comment

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

This is a good idea for a blog post. I have an SO question that covers this topic and has become my second most popular answer ever. I'm sure this post will similarly be one of our most trafficked posts.


In TypeScript, it's unusual to model data structures like this using classes. Typically, something like this is modeled using types. If you want to use TypeScript for this post, I'd suggest a more idiomatic style.

type Peripheral = {
  name: string;
}
type Mouse = Peripheral & {
  buttonCount: number;
  wheelCount: number;
  trackingType: "ball" | "optical";
}

There's one place where I don't think this example correctly reflects how polymorphism works. Your ideal schema for the Peripheral class includes additionalProperties: false. However, polymorphism actually depends on additional properties being allowed.

Imagine you have a function that takes a Peripheral. That function should be able to take things that extend Peripheral such as Mouse and Keyboard, but if your Peripheral schema doesn't allow additional properties, it can't do that. If you pass it a Mouse it would reject it because it has properties Peripheral doesn't allow like buttonCount. The type system doesn't allow you to use those additional properties in your function, but allowing them to be there is essential for polymorphism.

Comment on lines +96 to +122
```json
{
"$schema": "https://json-schema.org/draft/2020-12/schema",
"$id": "schema:mouse",
"$ref": "schema:peripheral",
"type": "object",
"properties": {
"buttonCount": { "type": "integer" },
"wheelCount": { "type": "integer" },
"trackingType": { "enum": [ "ball", "optical" ] }
},
"required": [ "buttons", "wheels", "tracking" ],
"unevaluatedProperties": false
}

{
"$schema": "https://json-schema.org/draft/2020-12/schema",
"$id": "schema:keyboard",
"$ref": "schema:peripheral",
"properties": {
"keys": { "type": "integer" },
"mediaButtons": { "type": "boolean" }
},
"required": [ "keys", "mediaButtons" ],
"unevaluatedProperties": false
}
```
Copy link
Member

Choose a reason for hiding this comment

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

Did you squish these into a single code block on purpose? If you did, that's fine. Just wanted to point it out in case it wasn't intentional.

Copy link
Member Author

Choose a reason for hiding this comment

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

I did. I prefer not to have multiple code blocks without text between them.

{
"$schema": "https://json-schema.org/draft/2020-12/schema",
"$id": "schema:mouse",
"$ref": "schema:peripheral",
Copy link
Member

Choose a reason for hiding this comment

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

This could be a relative reference. Do you prefer the full URI when using scheme:?

Copy link
Member Author

Choose a reason for hiding this comment

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

Since we haven't figured out how we would want something like this, I thought just the full URI would be fine. It's short enough in this case.

pages/posts/modelling-inheritance.md Outdated Show resolved Hide resolved
Co-authored-by: Jason Desrosiers <[email protected]>
@cloudflare-workers-and-pages
Copy link

cloudflare-workers-and-pages bot commented Sep 1, 2023

Deploying with  Cloudflare Pages  Cloudflare Pages

Latest commit: 39852f1
Status: ✅  Deploy successful!
Preview URL: https://441b9434.blog-cie.pages.dev
Branch Preview URL: https://gregsdennis-polymorphism.blog-cie.pages.dev

View logs

@gregsdennis
Copy link
Member Author

gregsdennis commented Sep 1, 2023

There's one place where I don't think this example correctly reflects how polymorphism works. Your ideal schema for the Peripheral class includes additionalProperties: false. However, polymorphism actually depends on additional properties being allowed. - @jdesrosiers

I included it because I see people put additionalProperties on abstract classes in order to avoid creating an instance of an abstract class, which is something that languages like C# will prevent you from doing. You can't instantiate just a Peripheral; you can only instantiate a Mouse or a Keyboard. So the thought is that by adding additionalProperties to the abstract class it prevents someone from satisfying the schema:peripheral constraints and just tacking on whatever else they feel like.

What they inevitably run into is that "inheriting" the schema doesn't overwrite the additionalProperties constraint.

I go on after the example to explain why this approach is wrong, and it becomes my first concession.

Copy link
Contributor

@benjagm benjagm left a comment

Choose a reason for hiding this comment

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

Congrats for this article. I love the tone, history flow and overall creativity.
So cool!

@gregsdennis
Copy link
Member Author

I'm going to leave the TS classes. It may not be the common way to do it in TS, but it still illustrates what's going on, and I'd like to have the snippets be more consumable than just for people who know C#.

@gregsdennis gregsdennis merged commit 566acf6 into main Sep 13, 2023
2 checks passed
@gregsdennis gregsdennis deleted the gregsdennis/polymorphism branch September 13, 2023 23:50
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants