qref icon indicating copy to clipboard operation
qref copied to clipboard

Refactor repetitions to a separate datastructure

Open mstechly opened this issue 1 year ago • 1 comments

As @dexter2206 mentioned in https://github.com/PsiQ/bartiq/pull/136#pullrequestreview-2419346221:

After reviewing this and the QREF's counterpart of this PR I'm more and more convinced that it would be better if we had a separate class representing "loop" or a "routine with repetitions" (or at least if we had it in Bartiq). This is because a lot of the logic we perform for validation and dispatching (with ifs) could be neatly hidden. For instance, we verify if routine with repetition has one child - but we could have an object that HAS to have one child (which would be validated by Pydantic). (...)

We should evaluate whether we want to introduce such a change in QREF natively or just in Bartiq.

mstechly avatar Nov 07 '24 12:11 mstechly

After some playing around with conversion code, the extra level of indirection that we have right now is definitely problematic and makes it harder to compare structures before and after conversion. Having a separate datastructure (e.g.: "RepeatedRoutine") which would have constraints of having only one child would not solve this issue, it would still be confusing.

I think we should consider having repetition field in any routine. I think the reason for not having that in the first place was that it made counting resources more tricky. However, perhaps we could get around it by having different types of resources or nested resource structure (with "resource subtypes")?

Not sure what the right solution is, but we should not have that extra level of indirection.

mstechly avatar Jan 02 '25 15:01 mstechly