When should I return error vs optional?

Brainstorming
1

As an exercise for learning Zig, I solve Advent of Code problems. In a recent problem (Advent of Code 2022, day 2, both parts), I came across the following scenario:

const OpponentMove = enum {
    ROCK,
    PAPER,
    SCISSORS,

    pub fn fromU8(value: u8) !OpponentMove {
        return switch (value) {
            'A' => OpponentMove.ROCK,
            'B' => OpponentMove.PAPER,
            'C' => OpponentMove.SCISSORS,
            else => Invalid.OpponentMove,
        };
    }
};

The problem required you to handle input for the classic game of Rock, Paper, Scissors. Naturally, this is a fallible operation because the input, while promised by AoC to be correct, could be invalid. At a glance, using errors feels like the go-to choice for representing this problem. Not only can it be handled ergonomically by the caller, but it composes nicely with the caller’s return type. For example:

pub fn part1(strategyGuide: []const u8) Invalid!u32 {
    // some code
    const opponentMove = try OpponentMove.fromU8(gameLine[0]);
    // some more code

While I’m quite confident that this code would pass the smell test and be considered idiomatic Zig, another option came to mind form the Rust world.

In a great video from the YouTuber Logan Smith, he shows that the Rust version of error, Result, are actually isomorphic to Option if and only if there is exactly one variant of the Result. This idea got me wondering:

When is it appropriate to use an optional instead of an error?

Here’s what my previous code snippet would look like when implemented with an optional:

const OpponentMove = enum {
    ROCK,
    PAPER,
    SCISSORS,

    pub fn fromU8(value: u8) ?OpponentMove {
        return switch (value) {
            'A' => OpponentMove.ROCK,
            'B' => OpponentMove.PAPER,
            'C' => OpponentMove.SCISSORS,
            else => null,
        };
    }
};

So far, it’s not to bad. Here’s what the call site could look like:

pub fn part1(strategyGuide: []const u8) Invalid!u32 {
    // some code
    const opponentMove = OpponentMove.fromU8(gameLine[0]) orelse return Invalid.OpponentMove;
    // some more code

Hmm… this is where I feel like the code is less idiomatic. While this version of the function still gives the caller a way to propagate up the error and thus not loose any information while handling errors, it feel quite a bit more verbose with the use of orelse.

How does the community feel about this alternative?
Is it more “idiomatic” to use the try key word? Or is my bias for playing code golf sneaking through?

I’d love to hear everyone’s thoughts and opinions on the subject :slightly_smiling_face:

2

If AoC says they don’t give bad input, I would be inclined to else => @panic(“bad input”) inside the switch. Then you don’t need to worry about it further up the call chain.

4 Likes
3

I’m of the opinion that any time you need to describe why something failed, you should return an error.

Think of an extreme example (I like this approach because it makes the principle obvious). Imagine having an HTTP client that could fail for a variety of reasons. Maybe it timed out, maybe it was a bad request, maybe the authorization failed… etc…

I could signal all of these with errors quite easily. Depending on the circumstance, I may even try the request again.

If all I got back was a null payload, what does that tell me? Optionals at some level are binary - they are null or not. In those cases, it makes perfect sense (think of searching for an element in an array - if it’s not in the array, it makes perfect sense to return null. In that case, nothing “went wrong” because the search itself was completed successfully). In other words, optional makes sense when something really is optional.

3 Likes
4

For small examples, I’m inclined to agree - no one really cares if the program gets shutdown. For general advice, this needs to be done with care.

For a system that needs to stay running, a @panic is usually not a good choice unless it’s testing an invariant (or additionally, if it is near the start of the program where damage won’t occur… it’s better for the panic to happen before the plane can take off as opposed to after). In some cases, it’s appropriate if the best thing to do is to close the program. It also makes code less reusable because it forces things to fail when they may be recoverable in other circumstances. (although, you can override panics with something like setjump and longjump but that’s another topic).

Again, not saying you’re wrong, just adding some context :slight_smile:

3 Likes
5

Absolutely agree - this would be a terrible solution to the problem in a different context!

I find that I often worry too much about the “best” way to handle a side problem like this, to the point that it eats away at my time for solving the real problem :sweat_smile:
For software that needs to be robust, it’s absolutely worth spending the time to properly flesh out a solution. And this is a brainstorm after all; the more discussions/solutions the better!

2 Likes
6

Only humans would care; the compiler probably sees them as pretty equivalent.

My personal take: “Optional” is sometimes but valid. “Error” is genuine failure and totally invalid.

Think about searching a string for a pattern.

How do you handle the pattern not being found? A pattern not being found really isn’t an “error”–it’s a bog standard result. You can return it as an error like the Python .index() method (which returns an exception), but it’s really not. The Python .find() method returns a position of -1, but that means you have to know that -1 is the “Invalid” indicator. “Optional” is just a much cleaner way to express that to other programmers.

By contrast, what should you do when your string search encounters invalid UTF-8? There is simply nothing intelligent you can do here other than signal “Error”. Something is wrong and you have no ability to deal with it.

Maybe you could even consider propagation. An “Optional” is simply an “Error” that the caller should very well be expected to deal with while an “Error” is something that even the caller may struggle to do something intelligent about and might simply need to be re-thrown.

In the end, though, it pretty much boils down to personal taste.

Note that this is probably more specific to Zig (and maybe Rust). Other languages may have a LOT more machinery that gets invoked if you are passing an “Error” around instead of an “Optional”. An example would be “traceback” info–I would expect that an “Optional” doesn’t invoke traceback machinery while an “Error” may very well do so.

3 Likes
7

This is a good branching-off point for one of the criteria which should be weighted in making this decision: is it library code, or application code?

Runeset exists to match UTF-8 encoded codepoints, and there are real performance implications involved in validating the UTF-8 structure while doing that. So it offers three families of match functions: match(One|Many), and the -AllowInvalid and -AssumeValid variants.

matchOne returns a ?usize: 0 for no match, null for invalid Unicode, and one to four depending on the size of the matched codepoint. The thinking is that application code using matchOne or matchMany, mostly doesn’t need to know why/how invalid Unicode was encountered, and if it does, can use the standard library (which I’ll get to in a moment) to find out.

matchOneAllowInvalid always returns a number: 0 is a non-match, it might be invalid, it might just not match.

matchOneAssumeValid turns the structural validity checks into assertions: this will panic in .Debug and .Safe modes, and disappear in optimized modes, while telling the optimizer that it’s allowed to assume that certain things will never happen.

I considered making matchOne throw when it hit invalid UTF-8, but I decided that the meaning of the function is “tell me how much of the next string matches” and that isn’t asking “also, tell me that it’s valid Unicode”. But, two things: first, bad things will happen if code uses the optimized path on invalid UTF-8, specifically, a truncation of a four-byte sequence into e.g. two bytes at the end of a slice, will read past the end of that slice. Also, unexpected structure can give false positives, because the AssumeValid code is allowed to assume that when it sees a three-byte lead character, the next two bytes will be follow bytes. In matchOne we have to check this anyway, so that no possible input will cause runeset to panic (without user opt-in), and so that the answer is always correct.

Second, “it is impossible to match this because it isn’t a codepoint” is in fact a different category from “this codepoint doesn’t happen to match this set”, and it’s a difference user code might care about. So AllowInvalid gives the option of treating it like any other failure to match, but in the base variant, it’s a null. If you need to know why it’s invalid, the null path can call the standard library, and catch the error it returns, with utf8Decode.

std.unicode.utf8Decode can throw four different errors. That’s a good interface for what it does. The function means “turn this slice into a codepoint”, so if that’s not possible then an expectation has been violated, and it’s correct to signal this with an error.

But in application code, I have been known to wrap that function in a helper which turns it into an optional. If, for instance, all I’m going to do with invalid data is increment an index and continue a while loop, an optional is cleaner. But an optional would be the wrong choice for library code, because it destroys information, and because failure to decode is legitimately an error state.

If you’re working on library code and on the fence about which way to go, you can consider having someFunction and maybeSomeFunction, where maybeSomeFunction just catches the errors from someFunction and returns a null instead. The Zig standard library favors as many small variations as it takes to provide a good interface, and the lazy compilation model makes it cheap, because if user code doesn’t touch some function, it won’t get linked into the binary (or indeed even semantically analyzed). This is a good model for other libraries to follow.

When you’re the end user of a function, it’s fine to reason that “it doesn’t matter why this fails, I’m always going to do the same thing, and besides, sometimes I know it won’t fail” and use an optional there, for the ergonomics of it. That sort of thinking is on less solid ground for a library.

1 Like
8

So many great comment! I’ll start from the top

Great minds think alike. This was actually my first solution I used and it worked just fine. As I mentioned at the top of the post, this is more of an academic exercise to flex some Zig muscles I haven’t had the chance to flex yet.

Well said! That’s what I was wrestling with when I wrote the optional solution: is it reasonable to get a null when mapping from a u8 to an enum? Probably not. Something definitely “went wrong” if you don’t get back an enum.

Agreed!

Good observation!

I hadn’t considered this. While this may be overkill for my toy example, I’ll keep an eye out for opportunities to apply this rule going forward.

2 Likes