pub struct Locker {
books: Arc<Mutex<HashMap<String, Book>>>,
}
...
fn get_book(&self, title: &str) -> Option<Book> {
let books = self.books.lock().unwrap();
books.get(title).cloned()
}
I clone here to avoid introducing lifetimes, which, well - adds a bunch of complexity (in this instance).
Books are small.
Question is, when is this considered bad practice? When our things are "not small"? Or - always?
How would you do this?
Not always. Only when they are relatively expensive to clone, where relatively expensive is really up to you, which I'm sure is an unsatisfying answer.
But for example, even if they were quite large but were only cloned infrequently, this might be fine in some cases. But in other cases, a size over 1KiB or any allocation at all by the clone may not be tolerated due to strict performance constraints.
In general, cloning in situations like this is very common.
Another way of looking at this is to ask, what is the alternative to cloning the Book when getting it from the HashMap? Every object must have a single owner. Here that owner is likely to be the HashMap. So the only way to avoid cloning the Book is to wrap the Book in an Arc (or Rc if only accessed by a single thread), which requires an extra allocation when adding the Book to the HashMap. So then there is a trade-off between the allocation to create the Arc, and whatever allocations are done when not using an Arc and cloning the Book. Which is done more frequently? And does it even matter, given your performance requirements? Only you can answer those questions.
You might want to check out concurrent hashmaps like DashMap
Perhaps you could use its view method to avoid cloning.
I appreciate the suggestion, but Rust is quite complex enough without bringing in libs for basic things like this. 
It's quite common to have a couple methods for getting the thing as a reference.
get -> &
get_mut -> &mut
And if the user requires ownership, they can always clone themselves at the call site.
books.get("title").cloned()
There's a tradeoff between performance (why am I cloning a string when all I need is a reference?) and convenience (why do I have to call .cloned()? I always want the String).
For something public and used as a dependency by others, I'd prioritise the performance and offer get, get_mut. For your own internal use prioritising convenience is nice, do what makes you happy!
I'm not aware of the conventional name for a method that gets and clones for you. get_owned or get_cloned perhaps? Probably not common because it's almost as verbose as simply calling .clone() yourself once it has a disambiguating name.
You can make it marginally more satisfying by saying that it's only an issue if profiling shows that cloning is a significant issue in your program (for whatever metric you're profiling - CPU time, memory utilization, wall clock time) such that the programmer time to remove the clone and use references or shared ownership is worth spending.
This topic was automatically closed 90 days after the last reply. We invite you to open a new topic if you have further questions or comments.