fix: avoid infinite recusion due to cyclic dependency#373
Conversation
Horusiath
left a comment
Horusiath
left a comment
There was a problem hiding this comment.
Technically we could change recursion to broad-/depth-first loop and be done with it, but it's ok. I only left little remark about unnecessary cloning.
collab-folder/src/folder.rs
Outdated
| txn: &T, | ||
| view_id: &str, | ||
| mut visited: HashSet<String>, | ||
| ) -> Vec<View> { |
There was a problem hiding this comment.
Technically since this method is recursive, it would be better to pass resulting &mut Vec<View> as accumulating argument instead of copying data over on each recursive step.
There was a problem hiding this comment.
Updated the method signature. Since there are significant number of changes now, i think it's better if i add some tests or test this end to end before merging.
There was a problem hiding this comment.
Technically you can still keep the public API unchanged, and move recursion and new params declaration to an inner (private) method. This should make it - more or less - a drop-in replacement.
khorshuheng
force-pushed
the
filter-out-traversed-views
branch
2 times, most recently
from
56ae552 to
e265484
Compare
khorshuheng
force-pushed
the
filter-out-traversed-views
branch
from
e265484 to
295c7c6
Compare
3 participants
There are some edge cases where there is a cycle in a folder tree, which can lead to stack overflow. To break the cycle, we add an additional condition such that the traversal stops when a view id has been traversed before.