Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Suggestion: Panic when appending to node that has self node as child. #78

Closed
mpawelski opened this issue Mar 4, 2022 · 0 comments · Fixed by #79
Closed

Suggestion: Panic when appending to node that has self node as child. #78

mpawelski opened this issue Mar 4, 2022 · 0 comments · Fixed by #79

Comments

@mpawelski
Copy link

Hi,
thanks for writing this nice library.
For some reason I thought I could use indextree as a graph structure. Later I realized that this library is just for tree structures. And I got such strange behavior:

        let arena = &mut Arena::new();

        let start = arena.new_node("start");
        let a = arena.new_node("A");
        let b = arena.new_node("b");
        let c = arena.new_node("c");

        start.append(a, arena);
        a.append(start, arena);
        start.append(b, arena);
        b.append(start, arena);
        assert_eq!(start.children(arena).into_iter().count(), 2);

        a.append(c, arena);
        c.append(a, arena);

        assert_eq!(start.children(arena).into_iter().count(), 2); //FAILED: count is 1

I guess doing "proper" validation to check if I'm building graph is difficult. For example this gives infinite look:

        let arena = &mut Arena::new();

        let a = arena.new_node("a");
        let b = arena.new_node("b");
        let c = arena.new_node("c");

        a.append(b, arena);
        b.append(c, arena);
        c.append(a, arena);

        assert_eq!(a.children(arena).into_iter().count(), 1);
        assert_eq!(a.descendants(arena).into_iter().count(), 3); // infinite loop

But in my case just panic when trying to "connect" nodes to each other would give me earlier feedback that I'm using this library in a wrong way. I propose that for second append the code below should panic (I hope it's easy to check it, I didn't look at the source code)

        let arena = &mut Arena::new();

        let a = arena.new_node("a");
        let b = arena.new_node("b");

        a.append(b, arena);
        b.append(a, arena); //IMO this should panic

        assert_eq!(a.children(arena).into_iter().count(), 1);
        assert_eq!(a.descendants(arena).into_iter().count(), 2); // infinite loop

BTW, the example code on https://crates.io/crates/indextree is not compiling (assert!(a.append(b, arena).is_ok()); line)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant