Order of internal asserts vs argument checking

1

Let's say one has these two pseudocode snippets:

Snippet 1:

fn do_something(&mut self, args) -> Result {
    if !is_valid(args) {
        return Err(error_information);
    }
    assert!(self.validate_internal_data_structures());
    return Ok(computation_with_args(self, args));
}

Snippet 2:

fn do_something(&mut self, args) -> Result {
    assert!(self.validate_internal_data_structures());
    if !is_valid(args) {
        return Err(error_information);
    }
    return Ok(computation_with_args(self, args));
}

I'd think verifying internal data structures (which could impact the correctness of the computation) should be done first (as in snippet 2), but this could lead to a situation where the object is borked but for which a panic is not triggered because the given arguments are invalid, suggesting that snippet 1 is better. (The verification here is an invariant that should always hold unless the code is buggy, and an ideal compiler would be able to eliminate these checks by statically proving they'd never fail.)

Which one would you consider better, and why? (Or would you consider something else better, e.g. verifying internal state both before and after the computation?)

2

Code that contains an assert! should remain correct if you remove the assert! outright. If you do, then both of those snippets are equivalent, and this is mostly a user friendliness issue.

It's up to you to decide if programmers working with your function would be better served about finding out that internal invariants have been violated (arguably a "you" problem, but a fatal one that only they can detect), or finding out that arguments don't meet requirements first.

Is there a way to eliminate the range of invalid arguments, for example by defining the type in a way that has no representable (or at least no constructible) invalid values?

3

In terms of "eliminat[ing] the range of invalid arguments", I agree that this is the approach I'd usually take, but this isn't always possible. In my case, I have a function that takes ranges as RangeBounds to allow code like self.do_something(a..b). I'd need to manually check that the range is not empty before performing operations using it, but I believe the benefit of being able to use native range syntax outweighs the small cost of additional argument validation.

4

This is very much not true. A lot of unsafe code uses assert! to check its invariants, and if you removed the assert! would be UB. An example is in String::truncate which uses assert! to check that the provided index is not into the middle of a character, then truncates the underlying byte vector.

You might be thinking of debug_assert! which is toggled on and off with cfg(debug_assertions). That is most commonly used for checking invariants which are assumed to always hold, but to improve the ability for fuzzers and tests to catch violations they are explicitly asserted while testing.

2 Likes
5

This topic was automatically closed 90 days after the last reply. We invite you to open a new topic if you have further questions or comments.