crates/doc: refactor inference module into shape module and friends
#1137
Conversation
jgraettinger
force-pushed
the
johnny/schema-infra
branch
from
d9b9b9e
to
a3498ca
Compare
|
|
||
| // Prune any locations in this shape that have more than the allowed fields, | ||
| // squashing those fields into the `additionalProperties` subschema for that location. | ||
| pub fn enforce_field_count_limits(slf: &mut Shape, loc: Location) { |
There was a problem hiding this comment.
Not an issue, but I am curious why this isn't in an impl Shape block, or rather what heuristic you're using to determine when to use a free function vs an impl. I had been using impls mainly for discoverability: we have a pretty big codebase, and being able to write my_shape. and have my IDE give me a list of functions I can call on Shape feels useful. Is it that we're not confident in this API and don't want to "expose" it?
There was a problem hiding this comment.
Is it that we're not confident in this API and don't want to "expose" it?
That's some of it, yes.
It's also orthogonal to what Shape is and does, and isn't core to it's function. Especially once you re-interpret widen to return "did this widen change the Shape anywhere?". widen is a core capability of Shape, while enforce_field_count_limits is business logic applied to a Shape. And a big distinction in my mind, is that we can define lots of different heuristics and trade-offs for when to prune stuff out of a Shape, but there's only one sensible widen operation.
Also, as a general strategy, I definitely recommend starting out with free functions and only attaching them to structs when it's really obvious they belong. It makes refactoring way easier, especially in the early stages of a subsystem when you're unsure how everything fits together.
As a discipline, I find a lot of engineers will jump too quickly into an object-plus-behaviors-oriented mindset and then get stuck when their initial conceptual modeling doesn't match evolving requirements. Free functions that you can refactor and move as-needed avoids that mental trap, because it defers bestowing "object-ness" onto what are, at the end of the day, just a pile of bytes.
doc::shape has a variety of other submodules, some of them public and others not, which focus on particular features and capabilities of Shape. No behavior changes, and no significant changes to tests, though I moved quite a few things around.
Update to use doc::shape and friends instead.
jgraettinger
force-pushed
the
johnny/schema-infra
branch
from
a3498ca
to
1619e92
Compare
See individual commits. No (intentional) behavior changes in this PR, it's just a refactor.
This change is