Model builder pattern #22
Conversation
|
Thanks Felix, you may have seen #23. Let's discuss which way is better at this point. Hopefully the astjson-to-rust work shouldn't be affected by the underlying data structure too much. |
src/common/ast/ast.rs
Outdated
| #[derive(Clone, Debug, PartialEq)] | ||
| pub enum Domain { | ||
| BoolDomain, | ||
| IntDomain(Vec<Range<i32>>), |
There was a problem hiding this comment.
This is fine for now, but at some point someone will have to write (or find) a lovely library for ranges of ranges -- at the moment the domains [0..3), [3..6), [0..3], [1..6) and [0..6) would all be different, despite them having the same underlying values. I'm sure at a later date there will be a demand for interesting + unioning such domains as well.
|
can you try merging main onto this pr @lixitrixi ? I tried to fix the rustfmt github action by using a fixed version number, just want to see whether that worked. |
|
I have removed the |
|
are we abandoninig this one or do you want to update it @lixitrixi ? |
|
@ozgurakgun Was the code structure PR passing rustfmt before you merged it? |
|
Was actually thinking... would it be better to add the methods like |
|
sure! if it was me I would write the astjson parser and see what kind of builder makes sense for that. |
That makes the most sense... I will close this and implement necessary model methods over on #32 as I work. |
This is a continuation of work from #16. Rebasing has proven difficult so I have based this branch on the current merged changes and manually copied my work over.