That is not quite enough, because then animal is an animal pointer &Animal instead of an owned animal. But if you just borrow out your animal you won't have that problem: (Note that the function now returns Result<&Animal, &'static str>).
fn find_by_name(&self, name: &str) -> Result<&Animal, &'static str> {
for animal in &self.animals {
if animal.name == name {
return Ok(animal);
}
}
return Err("Could not find animal")
}
This also makes some intuitive sense: If you only borrow the zoo, you can't give away ownership of the animals. They aren't your animals.
Not sure if this is right, but how I worked it out after a little learning. (This is the most complex thing I've thought through in Rust so far, so thanks for that.)
This avoids the detour via the index you do. Otherwise I am perfectly fine with your code. Here you can also define a mutable borrow by swapping .iter() out for .iter_mut().
Edit: Removed ref, thanks to @xfix for pointing that out.
Because it's still an iterator (filter returns an iterator that only has element that fit a given predicate). .nth(0) probably can be written in more clear way as .next(), as it returns the first item of iterator.
Using &self.animals and returning a &Animal make sense! That's what I wanted to figure out. I didn't know I had to use &self.animals (I assumed I already had the reference since I used &self in the function arguments).
I've also seen this _mut pattern — it makes sense to apply this here as well.
Thanks for the alternative implementations with .iter() and .filter(), although I don't see how this would improve readability or performance.
You should take note that add_animal can add more than one of the same exact animal, and you only search for the first animal found, by name. It probably makes a bit more sense to use a HashMap and either a Vec of animals with the same type, or make them unique-- then you can directly expose the get and get_mut methods of HashMap.
I would agree that it makes more sense to use a HashMap keyed on the name. This is a slightly contrived example that doesn't really represent the real project.
In the actual project I also need to have control over the ordering of Animals. So I should probably use something like Java's LinkedHashMap, which guarantees insertion ordering.
@fdb: You were asking about "how this would improve readability or performance". To be honest I think both styles are equally clearer for this example and you should feel free to keep using for loops. For loops are definitely more beginner friendly. But some people, me included, just fancy this iterator style. It feels clean, functional and when you have more complex iterators it is nice to compose them from simple blocks.
Performance wise, the great thing about Rust is that the iterator style isn't slower. The compiler is smart enough to reduce it to a your loop.
And about ordermap, I haven't used it myself but I believe it only guarantees the order when you add elements to it. Removing elements will mess up the order. (Someone please correct me on this if I am wrong)
Even using the _mut pattern, I can't change the value.
I made a scope so the borrowed animals only live inside. But I think I'm doing something wrong with strings. How would you set the value for .name. Or is this an issue with the default object property not being mutable.
Getting: cannot assign to data in a & reference
{
let ref rosie = zoo.find_by_name_mut("Rosie").unwrap();
println!("In my zoo, {} is a {}", rosie.name, rosie.species);
rosie.name = String::from("Bob");
}
let ref bob = zoo.find_by_name("Bob").unwrap();
println!("In my zoo, {} is a {}", bob.name, bob.species);
The _mut function is fine, but you throw mutability away by using let ref rosie instead of let ref mut rosie. Actually, you don't even need ref mut it works fine without.
{
let rosie = zoo.find_by_name_mut("Rosie").unwrap();
println!("In my zoo, {} is a {}", rosie.name, rosie.species);
rosie.name = String::from("Bob");
}
let bob = zoo.find_by_name("Bob").unwrap();
println!("In my zoo, {} is a {}", bob.name, bob.species);
And I agree with @ndusart, an option would be more in line with how Vec and friends do it.
Thanks; I was trying this out and it seems the innocuously placed brackets around the "let rosie" block are quite important, otherwise I get this error:
error[E0502]: cannot borrow `zoo` as mutable because it is also borrowed as immutable
--> <anon>:57:17
|
53 | let rosie = zoo.find_by_name("Rosie").unwrap();
| --- immutable borrow occurs here
...
57 | let koala = zoo.find_by_name_mut("Cynthia").unwrap();
| ^^^ mutable borrow occurs here
...
63 | }
| - immutable borrow ends here
Here's the updated example:
I'm not sure I understand why I can't have a mutable and immutable borrow at the same time. Is the Rust book the best place to learn about this?
There is a ‘data race’ when two or more pointers access the same memory location at the same time, where at least one of them is writing, and the operations are not synchronized.