Of course, if that code had been written in Rust, the compiler would have caught the bug... no tests necessary, and no need to stick to clever coding patterns and write your own wrappers.
I know she likes her C, but I wonder if she'll eventually come around, drawn by the better reliability.
This isn't C. At least denigrate the correct language.
The problem here is a flawed object design that requires external knowledge of when methods can be called. The fix is to detect invalid calls to value(), log/print to stderr, and call abort(). With a suitable test suite these logic errors will reveal themselves before a release build.
> The fix is to detect invalid calls to value(), log/print to stderr, and call abort().
That is what the code does:
> > Now, had that code ever run, it would have CHECKed and blown up right there, since calling .value() after it's returned false on the pass-fail check is not allowed.
Sure, it also makes sure that the check has been done before calling either .value() or .error(), but that isn't really relevant to the issue at hand: the program aborts if you call the wrong one of those two based on what the object holds.
> With a suitable test suite these logic errors will reveal themselves before a release build.
This is why I prefer Rust's approach with Result: the normal way of using it[0] means that I can't use it incorrectly. If I try to, it will be caught at compile time, and I don't need to write and maintain a test for something so stupidly trivial.
[0] Yes, I can use unwrap() and kill those guarantees. I make a habit of very rarely using unwrap(), and when I do, I write a comment above the line that details why I believe it's safe and will never panic.
The short answer is that both values and errors are usually better scoped to only the places where they should be used.
For one, Rust's unwrapping of values is done in one step, as opposed to a "check first, unwrap second".
if let Some(inner_value) = wrapped_value {
// ..do something with inner_value..
}
Or this:
let Some(inner_value) = wrapped_value else {
// compiler forces this branch to divert (return, break, etc)
};
// ..do something with inner_value..
This makes it so you can't check something and unwrap something else.
Second, for pulling out errors, you would usually use a match statement:
match my_result {
Ok(good_value) => {
// use the good_value; the bad_value is not available here
}
Err(bad_value) => {
// use the bad_value; the good_value is not available here
}
}
Or:
let good_value = match my_result {
Ok(good_value) => good_value,
Err(bad_value) => { /* return an error */ },
};
// bad_value no longer available here
This makes it so that you can't invert the check. As in, you can't accidentally check that it's an Err value and then use it as an Ok, because its inner value won't be in scope.
You also can't use an Ok value as an Err one beyond its scope, because the scope of the error and the scope of the good value are a lot more limited by how if let and match work.
What the C++ code is doing is repeatedly calling `value.unwrap()` or `value.unwrap_err()` everywhere, pinky-promising that the check for it has been done correctly above. There's some clever checks on the C++ side to make this blow up in bad cases, but they weren't enough.
I know she likes her C, but I wonder if she'll eventually come around, drawn by the better reliability.