Best practice to create a function that does not accept zero

1

Hello,

In one of my crate I recently created, I have this function:

fn divide(&self, n: usize) -> Portion<'_, T> {
    assert!(n != 0, "cannot divide into zero portions");
    Portion::new(self, n)
}

The problem with this function is that I do not offer to the user the possibility to handle the case where the input is zero.

Another solution could be to return an Option:

fn divide(&self, n: usize) -> Option<Portion<'_, T>> {
    if n == 0 {
        None
    } else {
        Some(Portion::new(self, n))
    }
}

I also recently learned about the NonZeroUsize type:

fn divide(&self, n: NonZeroUsize) -> Portion<'_, T> {
    Portion::new(self, n)
}

I am wondering what is considered as a best practice to handle this kind of situation:

  1. Using assert is similar to what is done when the operator / is used on integer types.
  2. The NonZeroUsize type makes clear to the user that the function does not handle inputs with value zero. Howover, the user will have to explicitly converts every usize into a NonZeroUsize when using it.
  3. I feel like the Option solution combines the worst of the two worlds: it is not clear that the function does not support inputs with zero value, my function still have to continue to handle the case where n is zero, and the user will also need to manage that when the function returns.

Is there a best practice in the Rust community about this kind of situation?

Thanks.

2

The question is: What do you want your program to do when zero turns up there:

  1. Just give up and quit because something that should never happen has happened and you have no sensible way to handle it. Basically a bug somewhere.

  2. Somehow continue running and handle that zero case. The NonZeroUsize solution makes it clear to the caller what is required. They have to take care of that circumstance. After all somebody does if you want to continue.

1 Like
3

Generally, the most common approach for such a function with a non-zero input is the assert! one.

If the most common usage is with an integer literal or another case where it is highly likely that the caller will already know the input isn't zero, the assert! option is likely the best option currently[1]. However, if a common use case is with a fully unknown n as input, then returning Option is a good idea and fully idiomatic; the caller can unwrap if the value is known nonzero.

It's probably also a reasonable idea to tag the function with #[inline] #[track_caller] so the assert can assign "blame" directly to the caller or be optimized out in most cases where the value is known nonzero to the compiler.

  1. This may change in the future, if/when it becomes possible to have integer literals become a NonZero<_> type after type inference. ↩ī¸Ž

2 Likes
4

What about a Result<_, YourError>? You could even further restrict the number, and have proper error variants for the caller to be matched on.

Alternatively, I've just recently encountered this refined crate, which allows creating new types from existing types with constraint(s) applied.

An example from its documentation 
use refined::{Refinement, RefinementError, boundable::unsigned::{LessThanEqual, ClosedInterval}};

type FrobnicatorName = Refinement<String, ClosedInterval<1, 10>>;

type FrobnicatorSize = Refinement<u8, LessThanEqual<100>>;

#[derive(Debug)]
struct Frobnicator {
  name: FrobnicatorName,
  size: FrobnicatorSize
}

impl Frobnicator {
  pub fn new(name: String, size: u8) -> Result<Frobnicator, RefinementError> {
    let name = FrobnicatorName::refine(name)?;
    let size = FrobnicatorSize::refine(size)?;

    Ok(Self {
      name,
      size
    })
  }
}

assert!(Frobnicator::new("Good name".to_string(), 99).is_ok());
assert_eq!(Frobnicator::new("Bad name, too long".to_string(), 99).unwrap_err().to_string(),
           "refinement violated: must be greater than or equal to 1 and must be less than or equal to 10");
assert_eq!(Frobnicator::new("Good name".to_string(), 123).unwrap_err().to_string(),
           "refinement violated: must be less than or equal to 100");
5

It's better for it to return a result and let the user handle it in case of an error, personally i don't want to use a crate that panics on most of errors for me, especially when you are trying to make fault tolerant applications.
Another solid option is to make both versions of the function,
strict_divide and divide
strict_divide panics on errors, while divide returns a result or an option, you can even make an unchecked version that assumes that the value would always be nonzero,
you can always copy the design of the rust premitives and how they handle multiple cases:
Check this: https://doc.rust-lang.org/std/primitive.u32.html#method.checked_div

1 Like
6

Thanks for all of your answer. By reading your answers, it appears to me that there is no idiomatic Rust for this kind of situation.

In my case, because I want my function to have a behavior similar to what is done in the standard library (in the function std::slice::chunks in particular), I will continue to use an assert for consistency.

Thanks.

7

The standard library provides what they guess is the most likely usage as the default and has alternatives where it makes sense. In this case it's up to your library to decide what makes sense, just make sure you document it!

8

My attitude is that if my code compiles and gets through clippy without complaint then it is "idiomatic Rust".

Personally I don't want an assert firing off inside some crate I am using in my project, unless it really is caused by an unexpected bug in that create and hence should be fixed by that crates maintainer.

9

One common pattern is using something() and try_something(). In this approach, the default something() may panic in rare cases, while the try_ variant (which returns a Result) is available for clients who need to handle potential failures, even if they are unlikely.

Another pattern is using something() and unsafe something_unchecked(). Here, the standard something() returns a Result because failure is expected to be handled. The unchecked variant is intended for cases where the caller is absolutely certain that failure won't occur and prefers using unsafe over .unwrap(). This unchecked version skips all checks.

10

Yes good pattern for helper crates