There is an opinion to avoid using return in Rust. It's a pretty much as goto statement and a coder should avoid using it. I have nothing against using goto, since my background is from Fortran. However, what is your opinion on return?
Compare the following fragments:
if let Ok(output) = output {
if output.status.success() {
let pos = output.stdout[0] == 43u8; // '+'
let mut hour : i16 = (&output.stdout[1] - 48) as i16;
hour = hour * 10 + (&output.stdout[2] - 48) as i16;
let mut min : i16 = (output.stdout[3] - 48) as i16;
min = min * 10 + (&output.stdout[4] - 48) as i16;
if pos {
hour * 60 + min
} else {
-hour * 60 - min
}
} else {
0
}
} else {
0
}
And the same fragment with return:
if let Ok(output) = output {
if output.status.success() {
let pos = output.stdout[0] == 43u8; // '+'
let mut hour : i16 = (&output.stdout[1] - 48) as i16;
hour = hour * 10 + (&output.stdout[2] - 48) as i16;
let mut min : i16 = (output.stdout[3] - 48) as i16;
min = min * 10 + (&output.stdout[4] - 48) as i16;
return if pos {
hour * 60 + min
} else {
-hour * 60 - min
}
}
}
0
Or maybe even better:
if let Ok(output) = output && output.status.success() {
let pos = output.stdout[0] == 43u8; // '+'
let mut hour : i16 = (&output.stdout[1] - 48) as i16;
hour = hour * 10 + (&output.stdout[2] - 48) as i16;
let mut min : i16 = (output.stdout[3] - 48) as i16;
min = min * 10 + (&output.stdout[4] - 48) as i16;
if pos {
hour * 60 + min
} else {
-hour * 60 - min
}
} else {
0
}
Note, there is not obvious how avoid returns in complex, nested match statements.
The match based solution looks a pretty elegant. Actually Rust match is the same level powerful like when in Kotlin. The filter based solution requires more digest for me. Anyway, there is a big chance that return can be avoided.
Nothing wrong with using an early return to exit from the function. The keyword exists for you to use it. That doesn't mean it can't be abused or overused, but that should be decided according to your taste on a case-by-case basis, not by some general sweeping policy. The knee-jerk "this is just goto" comments are occasionally applied to any imaginable control flow construct. Hell, people objected to C adding break and continue keywords for that reason! Those comments can just be discarded outright, because they don't contain any evaluation of pros and cons of different approaches, and don't understand why goto is considered problematic in the first place.
That said, I have issues with the rest of your example code.
&output.stdout[1] - 48 --- you're overusing magic numbers. That's entirely unnecessary and just makes your code harder to read. There is no reason to write 48 when you can instead write '0' as i16.
You don't handle parsing errors in any way. output.stdout[0] == 43u8; // '+' --- and what if output.stdout[0] == 143? Similarly, what if the expected digits are arbitrary ASCII symbols? Parsing untrusted data is hard. Rust makes it easy. You should utilize its capabilities instead of writing cryptic brittle code.
The digit parsing code should really be extracted into a separate function. In fact, Rust already has that function! You can parse integers, with proper error handling, out of slices.
let hour: i16 = &output.stdout[1..=2].parse()
.map_err(/* error conversion code goes here */)?;
let minute: i16 = &output.stdout[3..=4].parse()
.map_err(/* error conversion code goes here */)?;
Again, you don't handle any errors. You shouldn't be returning dummy values (0) if something goes wrong. Instead, use the proper error handling types, either Option<i16> or Result<i16, ErrorType>, depening on desired semantics (is the error something meaningful that could be handled, or provided with a better error message? then use Result). This would also allow you to remove the outer nesting level of pattern matching (if let Ok(..) = output), instead utilizing the ? operator:
let output = output.map_err(/* convert error */)?;
Here is roughly how I would rewrite your snippet, using anyhow for simple error handling (you can use different error types depending on your needs):
use anyhow::{anyhow, ensure, Context};
use std::io;
use std::process::Output;
fn parse_time(output: io::Result<Output>) -> anyhow::Result<i16> {
let out = output.context("subprocess failed")?;
// Can we provide a more specific reason?
ensure!(out.status.success(), "subprocess failed");
// You can handle parse error in a different way if you want, e.g. by matching
// on individual digits. But you need to handle it somehow.
let text = core::str::from_utf8(&out.stdout)?;
let parse_int = |s: &str| {
s.parse::<i16>()
.with_context(|| format!("`{}` is not a valid integer", s))
};
let hour = parse_int(&text[1..=2])?;
let minute = parse_int(&text[3..=4])?;
let total = hour * 60 + minute;
match text.as_bytes()[0] {
b'+' => Ok(total),
b'-' => Ok(-total),
c => Err(anyhow!("invalid sign `{c}")),
}
}
P.S.: Consider whether Duration could work for you. It would provide better type safety, and remove questions such as "does this function return minutes, hours or seconds". Note that Duration can represent only positive time spans, so you would need to handle negative spans in some other way (e.g. wrap it in enum SignedDuration { Pos(Duration), Neg(Duration) }).
Personally I haven't really seen any hardcore anti-return sentiment in the Rust community. It's rather the opposite; a few people have expressed that they don't like the "implicit return" at the end of a scope, and write out explicit returns.
My rule is pretty simple: If avoiding the use of return just for the sake of not using return, and the code suffers for it, then just use return instead. There's no valor in making the code less readable, or less maintainable, just for the sake of avoiding the return keyword.
But who am I to say .. I used to use goto in C, in particular to express early-exit-and-cleanup.
Although your reasoning is a good, you missed own point - every solution should be a case by a case basis. The rule of thumb - do not overcomplicate the code even for sake of being better.
A reason to have just one return at the end, because it's easy to overlook other returns especially in case of a long match or something like that. But I am with you on a case of having multiple returns even in the case when they are not clearly visible.
There's nothing overcomplicated here. It is bog standard parsing code, you're just not familiar with the idioms. That's just what proper error handling looks like. You can skimp on detailed error messages if you want, but the basic structure would be the same.
The language gives you many tools to make your code correct by construction. You should utilize them fully instead of writing Fortran in Rust. Placement of return is entirely insignificant compared to using proper types and covering all edge cases.
Try to avoid one fundamental mistake - my opinion is the right, all other opinions are wrong and I can teach you the right one. All solutions are right, and you always can choose any more suitable for your taste and needs. Even AI gives me several choices without pushing to use a single right one.
Way back in the day I was maintaining some C++ code. It was generally very well-written and adopted a philosophy of using single returns. However, I recall discovering one bug which would not have happened had he relaxed that requirement.
But, in general, I tend to go for single exit points unless the code would be simpler if I didn't.
Using the return keyword scarcely in Rust does not translate to single returns in other languages. Rust is an expression-based language, and as such the preferred way of returning from a function is by having an expression as the last statement.
