Q6: Fixing
Of the following fixes (highlighted in yellow), which fix best satisfies these three criteria:
- The fixed function passes the Rust compiler,
- The fixed function preserves the intention of the original code, and
- The fixed function does not introduce unnecessary inefficiencies
/// Gets the string out of an option if it exists,
/// returning a default otherwise
fn get_or_default(arg: &Option<String>) -> String {
if arg.is_none() {
return String::new();
}
let s = arg.unwrap();
s.clone()
}
1:
fn get_or_default(arg: &Option<&str>) -> String {
if arg.is_none() {
return String::new();
}
let s = arg.unwrap();
s.to_string()
}
2:
fn get_or_default(arg: &mut Option<String>) -> String {
if arg.is_none() {
return String::new();
}
let s = arg.as_mut().unwrap();
s.clone()
}
3:
fn get_or_default(arg: Option<String>) -> String {
if arg.is_none() {
return String::new();
}
let s = arg.unwrap();
s.clone()
}
4:
fn get_or_default(arg: &Option<String>) -> String {
match arg {
None => String::new(),
Some(s) => s.clone()
}
}
Answer
4
- Really about best-practices here
4
is a better, more idiomatic version of3
, especially because it requires ownership ofarg
which is restrictive and may not even be available- But I think
3
does fix the problem
- But I think
1
doesn't fix the problem2
... I'm not sure about ... but I don't think having a mutables
helps with the problem either (?)
Context: The combination of is_none and unwrap here is a Rust anti-pattern, since a match combines the two functionalities and automatically deals with pushing the reference &Option into the interior to produce &String. Therefore the match solution is the most idiomatic, and passes the compiler without changing the intended type signature of the function.
The solution of changing &Option to Option is not desirable because it requires the caller to provide ownership of their option, which is a far more restrictive API.