-
-
Notifications
You must be signed in to change notification settings - Fork 119
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
Improve validation infrastructure #2157
Comments
I've started working on this. |
Progress here has been slow, as this issue is more of a side project for me, but work is ongoing. I've defined how validation checks work under the new infrastructure, and just ported the second validation check (#2277), and I'm happy with how that works. From now, this is mostly a matter of porting the rest of the checks to the new infrastructure, then figuring out how to port the per-object Re-tagging this issue, to reflect that it's no longer about planning, but actual implementation. |
I've been picking up the pace here again this week. See pull requests referenced above for details. Most of the validation checks have been ported to the new validation infrastructure! Just finished porting all shell validation tests, and only sketch and solid validation tests are left. |
Validation is an important part of Fornjot. The validation code checks shapes for validity, surfacing issues that would lead to problems later on, for example an invalid mesh when exporting to STL or 3MF.
The validation checks are currently organized per-object (each check checks a specific type of object), and they are all structured about the same. I have an idea for updating the validation infrastructure, organizing it per-check, which would have some advantages.
Concept
This concept essentially consists of two parts:
1. There's one type per validation check
Validation checks (of which there are multiple per object) are currently represented as methods on the object-specific validation error type. Following the new concept, each validation check would be represented by a dedicated type.
I think it might make sense to have a dedicated module per validation check, instead of a module per object, as it is right now. Validation check could then also be shared among several objects. For example,
Sketch
andSolid
have related checks that make sure objects that shouldn't be referenced by multiple other objects aren't, for example, and those could be shared under that new model.Open question: Would "validation check" be the best name for that category of type, or would we call them "validation error", since they contain error information?
2.
Validate
trait is parameterized per validation checkThe updated validation trait could look something like this:
And it would be implemented per-check like this:
Open questions:
ValidationError
in the trait (with theT: Into<ValidationError>
as specified), or does the trait method takeerrors: &mut Vec<T>
and we do the conversion elsewhere?Validate
implementation for an object. Where would we have code to run all validation checks for an object in the future? Maybe there's two levels of traits?Validate<T>
andValidateAll
? If so, the latter could also do the conversion intoValidationError
.Advantages
This concept would have a few advantages:
cargo doc
output, to document each validation check in as much detail as required. The documentation of the objects themselves would no longer be burdened with this, and could instead just reference the validation checks, adding context where necessary.Next Steps
There are still open questions, so this will require more thought and experimentation. Therefore, I'm opening this as a planning issue.
I probably won't work on this for now, so if anybody's interested, feel free to speak up!
The text was updated successfully, but these errors were encountered: