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

Remove pointers from children #28

Closed
douweschulte opened this issue Jan 3, 2021 · 2 comments
Closed

Remove pointers from children #28

douweschulte opened this issue Jan 3, 2021 · 2 comments

Comments

@douweschulte
Copy link
Owner

After some thought I think that the pointers from children to its parent are suboptimal. And completely removing them would solve some issues I foresee in the future (eg .remove() on an element while iterating...). So I guess I should vote in favour of removing them altogether, but at the same time I like the idea of having acces to some information, like the backbone() function. But that could be solved by saving a boolean in the Atom. And I guess that function (and some like that I would like to provide) are much less important then the safety and stability of the library.

So removing all pointers and the functions around it would be my proposal, but @TianyiShi2001 I would like some input if you have some? Do you think those pointers serve a purpose or are they just forced by me as I wanted to copy CCTBX?

@TianyiShi2001
Copy link
Contributor

I've always been in favour of removing the pointers because 1) I haven't yet encountered any task that absolutely requires a pointer to the parent (because a reference to the parent is usually implicitly available when accessing the child, for example in a nested for loop), 2) using raw pointers to implement doubly linked trees doesn't look idiomatic (usually this is implemented with Rc<RefCell<T>> and Weak<RefCell<T>> in Rust, and indeed in cctbx this is also implemented with the corresponding counterparts, shared_ptr and weak_ptr)

@douweschulte
Copy link
Owner Author

Okay then that is solved. The pointers can be removed. Thanks!

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

No branches or pull requests

2 participants