Hi,

I frequently find myself writing checks for things that, in all likeliness, will never happen (and if they did, would be somebody else's fault). While I think this is great in some cases, in others it leads to writing lots of redundant and inefficient code.

For example, in my reflection system you can set and get the values of reflected Properties, like so:

void PropertyInfo::SetValue(Variant owner, ImmutableVariant value)
{
    if (owner.GetType().IsCastableTo(*_ownerType) && value.GetType().IsCastableTo(*_propertyType))
    {
        _setter(owner.GetValue(), value.GetValue());
    }
}

As you can see, I'm checking the validity of the arguments before passing them to the setter. The problem with this is that in all likeliness, you wouldn't have gotten this property through anything other than the type information of "owner", in which case the check on "owner" is redundant. This kind of stuff appears all over the place, and not only is it becoming a pain in the ass to write, but it's frequently just reaffirming what I already know.

Anyway, I can see a few solutions to this:

1) Keep the check in the form of an "assert", so that it gets compiled down to nothing in Release mode.

2) Change the arguments to "void*" and "const void*", respectively; indicating that the arguments are not checked, since that is the caller's responsibility.

3) Redesign the way this is used so that the check is never needed (so something like UE4's "TFieldIterator").

I'm really leaning towards option 2 in this case, since Reflection is only going to be used in a few tightly-controlled portions of the codebase; but I think this is question applies elsewhere as well.

What do you think?

Your effort of writing the checks is wasted, because you silently ignore errors, which is a bad habit and will lead to other bugs, which are more difficult to find.

1) If it can't possibly go wrong unless the internal code is wrong, you assert.

If user errors can happen and you want to give users a chance to recover, you might want to throw an exception.

2) That is bad, because then the compiler can not help with compile time type checks and you get difficult to find runtime memory corruption or segfaults.

3) Making it so it can't get wrong is always best, but is sometimes difficult.

In the past, I've taken advantage of some special macros which are a combination of an if-check and an assert:

#ifdef _DEBUG
#define MUST_BE_TRUE(expression) if ((expression) || (Assert(#expression), false))
#define MUST_BE_FALSE(expression) if ((expression) && (Assert(#expression), true))
#else
#define MUST_BE_TRUE(expression) if (expression)
#define MUST_BE_FALSE(expression) if (expression)
#endif

In Debug, an assert will be raised in the unexpected case, but in Release you can still handle both cases.

Using asserts for code that shouldn't go wrong. For example: pointers that can't ever be NULL/invalid because they're allocated in the constructor. Code changes, and guarantees that exist today probably won't tomorrow. It's common that you'll have enough on your plate that you'll occasionally overlook some piece of code that relies on conditions no longer guarantee this code. Assert's good for these cases.

The general rule I go by is to assert on all exposed mutable state that may be left in a unexpected form by some combination of method calls. These assertions are usually clumped up at the top of functions, or sometimes top of loop bodies, in a big brick. Even if the assertion never gets tripped, it's still a nice thing to have there for the purpose of establishing a set of assumptions that are made during the authorship of the function body, and serve a a functional form of documentation. In languages that feature code contracts, I replace those assert-bricks with contracts instead. There is actual english-style documentation as well, but the assertions are for added specificity

In all, somewhere in the range of 20-25% of function-body lines of code are assertions on average. Often times the assertions are in place before the useful part of the function body is even written.

I can tell you though, spending the time bloating my code by 20-25% takes a bit of extra time, but that time is recouped many fold over by saved time in debugging.