c++exceptionassertdrypreconditions

if-throw precondition check effectiveness and the DRY principle


A lot of internet resources insist on checking preconditions in API functions via if (something_is_wrong) throw Exception{} instead of assert(!something_is_wrong) and I see some good points in it. However, I'm afraid such usage may result in doubling the same checks:

void foo(int positive) {
  if (positive < 1) {
    throw std::invalid_argument("param1 must be positive");
  }
}
void caller() {
  int number = get_number_somehow();
  if (number >= 1) {
    foo(number);
  }
}

will probably execute like

int number = get_number_somehow();
if (number >= 1) {
  if (number < 1) {
    throw std::invalid_argument("param must be positive");
  }
}

unless the call will actually be inlined and optimized by cutting out one of the ifs, I guess. Besides, writing the check twice (in foo() and in caller()) might be violating the DRY rule. Therefore, maybe I should go for

void caller() {
  int number = get_number_somehow();
  try {
    foo(number);
  } catch (std::invalid_argument const&) {
    // handle or whatever
  }
}

to evade repeating myself with those precondition checks, providing a bit of performance and a lot of maintainability in case of a function contract change.

However, I can't always apply such logic. Imagine std::vector having only at() but not operator[]:

for (int i = 0; i < vector.size(); ++i) {
  bar(vector.at(i)); // each index is checked twice: by the loop and by at()
}

This code results in extra O(N) checks! Isn't it too much? Even if it is optimized out the same way as above, what about such situations with indirect calls or long functions, which probably won't be inlined?

So, should my program be written according to the rules below?

If not, why?


Solution

  • So, there are two separate things you are talking about: DRY and performance.

    DRY is about code maintenance and structure, and doesn't really apply to code you can't control. So, if the API is a black-box, and there happens to be code inside of it that you can't change, but you need to have separately, then I wouldn't think of it as not DRY to repeat it in your code. Y is Yourself.

    But, you could still care about performance. If you measure a performance problem then fix it with whatever makes sense -- even if it's anti-DRY (or it's ok if it is).

    But, if you control both sides (the API and the client) and you really want a pure, no-repeat, performant solution, then there's a pattern something like this pseudocode. I don't know the name, but I think of it as "Proof Providing"

    let fn = precondition_check(myNum)
    if fn != nil {
        // the existence of fn is proof that myNum meets preconditions
        fn()
    }
    

    The API func precondition_check returns a function that captures the myNum in it and doesn't need to check if it meets preconditions because it was only created if it did.