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

[Feature Request]: fast flag cleanup function #17

Open
jay-herrera opened this issue Nov 21, 2024 · 2 comments
Open

[Feature Request]: fast flag cleanup function #17

jay-herrera opened this issue Nov 21, 2024 · 2 comments
Labels
question Further information is requested

Comments

@jay-herrera
Copy link

Currently, for the operations remove() and update(), there is the fast flag, which defers cleanup of empty subnodes. If I'm reading the docs correctly, the expectation is that remove/update would be called with the fast flag set to true for the entirety of one "batch" of cleanup, except the last one, where it would be set to false, and the cleanup of empty subnodes would happen all at once.

This is great if you know ahead of time how many removals that you're going to perform, but consider the case where inside of a loop, you conditionally remove an object from the tree. In that case, you don't know which removal will be the last one, so you can't easily determine which remove call should have the fast flag set to false.

This could be fixed if there was a cleanup() function, that you could run at the end of the loop.

@timohausmann
Copy link
Owner

timohausmann commented Nov 27, 2024

tree.join() should actually do what you are looking for (docs)

If possible, let me know if that works for you, then I can add this to the docs and examples.

I thought about renaming it to cleanup but then we have clear() and cleanup(), potentially confusing, any other ideas?

I see the usefulness, maybe the fast flag should be deprecated and calling this method be recommended.

@timohausmann timohausmann added the question Further information is requested label Nov 27, 2024
@jay-herrera
Copy link
Author

That's actually very helpful! I think including in the documentation specifically that it could be called after a fast flag to clean up subnodes would be helpful, and might be enough. As for renaming, I am partial to verbose and specific function names, something like cleanupSubnodes(), but I'm not a library dev, so feel free to take the suggestion with a grain of salt.

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

No branches or pull requests

2 participants