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

Review ui::App for its use of &str/String #4

Open
jczuchnowski opened this issue Apr 26, 2020 · 1 comment
Open

Review ui::App for its use of &str/String #4

jczuchnowski opened this issue Apr 26, 2020 · 1 comment

Comments

@jczuchnowski
Copy link
Contributor

No description provided.

@jczuchnowski jczuchnowski changed the title Review ui::App for its use of str/String Review ui::App for its use of &str/String Apr 26, 2020
@mtsokol
Copy link

mtsokol commented Sep 10, 2020

Just started learning so I might be all wrong

In app.rs:

I've started looking at the code and I think that e.g. in main event loop for FetcherResponse::FiberDump event to pub fn replace_fiber_dump we could pass &Vec<T> or even just slice &[T](according to reddit comment) as we don't mutate those Fiber structs but only traverse it for creating tree and retrieving label & dump strings. [EDIT] Right now I think that in this case it doesn't matter as it's the only thing happening to those Vectors.

And the same for tree_list_widget and list_tree_nodes for passing references without ownership as format! accepts &str so we don't need to pass an owned variant.
(I see that in this snippet we end up creating number of copies, like for fib_labels)
Then we don't need to instantiate UIFibers with label and dump copies which are then retrieved in the next lines.
At the end append method for Vec accepts a reference which gets emptied there so it think whole replacing fiber dump could be done without copying label and dump strings with clone() and to_owned().

As I understand we use String if we plan to append/change ownership it and for immutable content access we should only pass &str and slices &[T] (although slice can be mut).
WDYT? I might got it wrong completely.

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