Is it a good idea to use Result objects as function arguments?

Is defining functions such as bar to do cascaded calls like the one in main a good idea ?

fn foo(x: i32) -> Result<i32, Error> {
    //...
}

fn bar(x: Result<i32,Error>) -> Result<i32, Error> {
    //...
}

fn main() {
    let x = foo(2).unwrap();
    let y = bar(foo(2)).unwrap();
}

I have seen few(very rarely though) rust libraries(crates) where such function arguments were defined, so i was wondering if this is a good practice in general ?

Thank you,
Pradeep.

only if you plan to have different behavior depending on whether it's Ok or Err. If you only handle the err or the ok case, then you should create a function only for that type

3 Likes

@oli_obk ok, makes sense. But is it ok to just do unwrap() on the function version that takes Result type and let the program crash in case the unwrap didn't result in an Ok? I don't feel like it is correct way of handling errors, and had an argument with one of my friend regarding this behavior, hence this question on the forum.

It highly depends on what you want to get from the function.

Generally Result type defines a "maybe failed result", forcing you to take care of errors.
That is it makes you do something about error to handle it before obtaining real result of a computation.
Passing Result type intact into another function alleviates this goal, so I'd say this is not a general practice.

I can think of only a couple of situations when passing Result as a parameter into a function can be used:

  • a combinator function/method which combines several results of upstream functions or transforms them somehow, like callback for Iterator::find, Iterator::filter etc for I: Iterator<Item=Result<_, _>>,
  • a generic error handler, which abstracts error handling somehow (e.g. log, analyze error and panic if error is too serious, and then extracts Ok value).
4 Likes

If youre just going to unwrap it without doing anything special on Err case, I would not take a Result, and only take the T from Ok. That way, you can use and_then for nice chaining.

fn foo(x: i32) -> Result<i32, Error> {
    Ok(x)
}
fn bar(x: i32) -> Result<i32, Error> {
    Ok(x)
}

fn main() {
    let x = foo(2);
    let y = foo(2).and_then(bar);
    println!("x: {:?} -- y: {:?}", x, y);
}

playpen

3 Likes

I am the friend @9prady9 was referring to. Let me try to explain the issue. We are making a wrapper for a GPU accelerated scientific computing library. Our goal is to make a performant and easy to use API.

One of the issues that has recently popped up is the way Rust handles error handling. Most of the functions in our API perform large number of operations, allocate memory, performs bound checks etc. so we would like to return errors for everything. The problem we are finding is that the the API has become verbose when we added Result return types for all of the functions.

I had suggested that we apply traits to each function which will handle both the Result and other data structures. If a Result is passed we will simply unwrap it otherwise we will forward the data structure to the underlying implementation. This will simplify the API and give the user the flexibility to handle the error if necessary. Here is an example:

impl Convertable for Array {
    fn convert(&self) -> Array {
        self.clone()
    }
}

impl Convertable for Result<Array, AfError> {
    fn convert(&self) -> Array {
        self.clone().unwrap()
    }
}

pub fn add<T: Convertible, U: Convertable> (lhs: T, rhs: U) -> Result<Array, AfError> {
    let lhs_arr = lhs.convert();
    let rhs_arr = rhs.convert();
    // Perform computations on arrays
}

I don't see any issues with this type of API because that is essentially what you are doing with you call the unwrap function anyway. The user still has the ability to handle the errors but the API does not force them to write unwrap after each and every call. Our library is regularly used for prototyping ideas and algorithms and forcing the user to handle errors for each call seems excessive that type of work.

Umar Arshad

@DanielKeep Can you please provide your inputs on what umar explained above.

I find it weird not to see a single mention of the try! macro here. Has it been overlooked? Normally you'd almost never unwrap a Result.

well, try! is more about returning Results than accepting them.

It seems try! (and a hypotetical "?") are often associated with cascaded calls.

let y = try!(bar(try!(foo(2));
1 Like

I think you should force the user to make use of unwrap().

An unwrap() indicates that there should be error handling, even if it just consists of writing out the error message.

If you don't make it that obvious some people will experience crashes and then blame the developers (you) because your crate caused a crash.

Not entirely sure why I'm specifically being summoned, here... but alright.

My concern with this is that if a function accepts a Result, the last thing I would expect is for it to just unwrap it, especially if it also returns a Result. I mean, ideally you shouldn't be calling unwrap at all, especially not in a library.

Aside: I feel like "thou shalt not panic in thine libraries" should be written in huge, fifty foot high burning letters on the side of a mountain somewhere. There is nothing more frustrating than having a failure in a component that you just can't practically isolate.

I appreciate that Rust error handling can get really verbose, but I think that should be taken as motivation to provide simpler constructs in the language. I think this is one of those cases where the language absolutely needs to be convenient; if correct error handling is too hard, people start coming up with wacky work-arounds. :stuck_out_tongue:

Also, why not go the other way around? Have the trait convert into a Result, then try! that as the first step in the function? The error propagation is still there, even if it's a lot less efficient.

sigh I really, really wish we had suffix ?. Or even just method macros!

Anyway, summary since I only just woke up and may have rambled a bit: I don't recommend doing this; it's subverting reasonable expectations and making your interface look less panicky than it arguably is. If there's a pain point, that should be use to motivate change in the language.

2 Likes

FYI for those who might not know, the ? suffix was part of an earlier RFC which basically proposed foo()?.bar()?.baz() could replace try!(foo()).try!(bar()).try!(baz()) (I'm not certain try! can be chained directly like that but that's the idea); the goal being better ergonomics.

No, it would be try!(try!(try!(foo()).bar()).baz()) which requires you to basically read the expression like a tennis match to make sense of it. It's basically the whole reason I wrote a method-macro RFC (never submitted mine; I believe someone else did one independently).

1 Like

@everyone thank you for the feedback.

@DanielKeep Sorry for the summoning :stuck_out_tongue: I am relatively new to the rust community and didn't know who to ask for a experienced advice. I knew you from another post i made earlier about Error handling in rust and hence asked for you again :smile: thanks once again