Conversation
coriolinus
left a comment
coriolinus
left a comment
There was a problem hiding this comment.
Great idea! I'm a little surprised that you implemented the builders manually--both derive_builder and typed_builder are quite good--but this lets you build in a constant context, which could definitely be convenient.
| #[serde(skip_serializing_if = "Option::is_none")] | ||
| pub absolute_snapping: Option<bool>, | ||
| #[serde(skip_serializing_if = "Option::is_none")] | ||
| pub position_relative_to_grid: Option<Position>, |
There was a problem hiding this comment.
Please undo non-semantic changes here, where you moved the location of position_relative_to_grid.
| pub fn new(x: f64, y: f64) -> Self { | ||
| Self { | ||
| x: R64::new(x), | ||
| y: R64::new(y), | ||
| } | ||
| } |
There was a problem hiding this comment.
| pub fn new(x: f64, y: f64) -> Self { | |
| Self { | |
| x: R64::new(x), | |
| y: R64::new(y), | |
| } | |
| } | |
| pub fn new(x: f64, y: f64) -> Option<Self> { | |
| Some(Self { | |
| x: R64::try_new(x)?, | |
| y: R64::try_new(y)?, | |
| }) | |
| } |
Requiring explicit error handling is friendlier than a behavior which sometimes panics on invalid input, except when compiled in release mode, in which case it silently does the wrong thing.
|
Honestly using a library never crossed my mind, I just used a regex to build the functions. Maybe don't merge this as is so I can take a look at doing it automatically instead, then I can apply it to more of the objects. |
|
One thing to mention: if you do add a proc-macro dependency such as either of the crates mentioned above, please make it an optional dependency behind a feature gate ( |
|
After looking over the possible libraries, it doesn't look like any of them can create |
|
No problem, I did not mean to imply that deriving the builders was mandatory. I can approve once you've addressed the other comments. |
2 participants
This makes it substantially easier to programmatically create entities using a builder pattern, allowing only the fields in use to be specified.