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 extra environments #120

Open
dan-knight opened this issue Mar 8, 2024 · 0 comments
Open

Remove extra environments #120

dan-knight opened this issue Mar 8, 2024 · 0 comments
Labels
future Planned future features refactor

Comments

@dan-knight
Copy link
Collaborator

dan-knight commented Mar 8, 2024

Related to #121

A new environment clone.out is created and passed around throughout the process of preparing tree data and creating tree grobs (for context).

This approach seems to have been chosen to imitate the behaviour of mutable data structures in other programming languages. This is a valid concern, as R's default behaviour has the potential to create numerous full copies of the input data. With large trees, this would be hugely memory inefficient.

However, this non-standard pattern will be unclear to anyone reading the code that is not completely familiar with the full codebase. I think that the same benefits could be achieved without a new environment through restructuring the data prep and grob creation function calls. Specifically, taking a more "step-by-step" approach (rather than using deeply nested functions) would improve both the readability and the maintainability of the codebase as a whole.

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

No branches or pull requests

1 participant