Add id validation for entity parsing - #86
Conversation
| let has_id = obj.fields.iter().any(|field| field.name == "id"); | ||
| if !has_id { | ||
| return Err(anyhow!( | ||
| "No 'id' field found on entity {}. Please add an 'id' field to your entity.", | ||
| name | ||
| )); | ||
| } | ||
|
|
There was a problem hiding this comment.
My preference would be to have this in a separate validation function rather than during parsing. But it accomplishes the same goal so will leave it up to you.
It should just be clear when we need another validator where it should go so that we don't end up with distributed validation.
Perhaps for example we implement a method on Entity called "validate_has_id_field" or something. That takes an entity and returns a result of entity so we can chain validators like you can see in some other places.
Then have a root constructor function that runs all needed validators.
There was a problem hiding this comment.
My preference would be to have this in a separate validation function rather than during parsing. But it accomplishes the same goal so will leave it up to you.
I personally prefer keeping it in the parser, than having a separate validation function. https://lexi-lambda.github.io/blog/2019/11/05/parse-don-t-validate/
But, what could be good is to move the logic to validate_has_id_field as you said, and use it in the entity creation function to improve readability.
There was a problem hiding this comment.
I personally prefer keeping it in the parser, than having a separate validation function.
I get the point but what I'm suggesting is essentially still in the parse step if validation occurs in a constructor function. That way you still keep the check before the type can be created. It's more similar separating lexing from parsing. I'm essentially just saying we should add it in a way that is obvious what the pattern is for the next person that comes along to add a constraint.
There was a problem hiding this comment.
I see. Do you mean to put it where the Self is actually created?
JonoPrest
left a comment
JonoPrest
left a comment
There was a problem hiding this comment.
Leave it up to you but take a look at my comment 👍🏼
2 participants
No description provided.