• 10
• 10
• 12
• 12
• 14

# Safety vs Efficiency

This topic is 989 days old which is more than the 365 day threshold we allow for new replies. Please post a new topic.

## Recommended Posts

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?

Edited by Salty Boyscouts

##### Share on other sites

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.

##### Share on other sites

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.