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

Examples hint the unhandy usage of static variables, leaving ownership to others #48

Closed
peter-scholtens opened this issue Apr 24, 2023 · 4 comments

Comments

@peter-scholtens
Copy link
Contributor

When trying to modify both examples as a novice exercise, I stumbled onto the fact that static string slices are used. This immediately goes wrong when I simply move the creation of the cache to a separate function, e.g. in the async case:

fn new_async_cache() -> AsyncCache<&str, &str> {
	let c : AsyncCache<&str, &str> = AsyncCache::new(12960, 1e6 as i64, tokio::spawn).unwrap();
	c
}

The compiler will now rightfully complains:

fn new_async_cache() -> AsyncCache<&str, &str> {
                                   ^     ^ expected named lifetime parameter
                                   |
                                   expected named lifetime parameter

Using references to string slices leaves the responsibility ownership to others: I would expect the cache takes up ownership, why create it otherwise? Do you accept a patch where I modify the structures to String types for both key and value?

@al8n
Copy link
Owner

al8n commented Apr 24, 2023

Hi, in Rust, &str has no ownership, you should use something like AsyncCache<&'static str, &'static str> or AsyncCache<String, String>.

@peter-scholtens
Copy link
Contributor Author

Indeed. If the original owner goes out of scope, the cache points to an empty spot, which the compiler indeed detects. Your suggestion to use static indeed solves the problem as far as the compiler concerns, but effective points to strings in the memory of the executable: I expect that is not the intention of making this software cache? That's why I said in the title that the examples hint to use static variables. It would be nicer to demonstrate use of heap stored variables, but don't feel obliged if you think the example becomes too complicated.

@al8n
Copy link
Owner

al8n commented Apr 24, 2023

Indeed. If the original owner goes out of scope, the cache points to an empty spot, which the compiler indeed detects. Your suggestion to use static indeed solves the problem as far as the compiler concerns, but effective points to strings in the memory of the executable: I expect that is not the intention of making this software cache? That's why I said in the title that the examples hint to use static variables. It would be nicer to demonstrate use of heap stored variables, but don't feel obliged if you think the example becomes too complicated.

Yeah, you are right, it is not a good example. I am not aware of that because it was so long since I wrote the example. Thanks for pointing it out!

@al8n
Copy link
Owner

al8n commented Apr 24, 2023

Do you accept a patch where I modify the structures to String types for both key and value?

I really appreciate it if you can help me modify it.

al8n pushed a commit that referenced this issue Apr 24, 2023
* Cache takes ownership of input data

The cache example now owns the data, and not static references, as mentioned in #48 Also variable names are modified to explain their usage.

* Update sync_example.rs

The cache example now owns the data, and not static references, as mentioned in #48 Also variable names are modified to explain their usage.
al8n pushed a commit that referenced this issue Oct 17, 2023
* Cache takes ownership of input data

The cache example now owns the data, and not static references, as mentioned in #48 Also variable names are modified to explain their usage.

* Update sync_example.rs

The cache example now owns the data, and not static references, as mentioned in #48 Also variable names are modified to explain their usage.
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