Multiple Layers of Argument Checking

[jehugaleahsa]

Hello:

When we write our applications, we write them in distinct layers.
Because each layer is tested individually and can be used by many
other code sets, we have been implementing argument checking at every
layer.

However, this just seems like more tests that we have to write. I know
that most of the time the arguments will be valid because they are
sent in by other layers that have also been tested. Some times the
next layer above won't do anything with the arguments at all except
pass them to the next layer down.

So, the question is, do we keep writing checks at every level or do we
use a different approach? Is there a standard for this type of thing
where you work?

Some times I feel like throwing argument checks in at all layers since
I know that they cost relatively nothing. This would at least keep
things consistent at all layers of the system. Plus, each layer being
so reusable, it might be the "safest" alternative. However, the
thought of writing all those tests and all that code to perform a
check I can almost 100% of the time guaruntee will never be hit is
hard to stomach.

 

[Pavel Minaev]

When we write our applications, we write them in distinct layers.
Because each layer is tested individually and can be used by many
other code sets, we have been implementing argument checking at every
layer.

However, this just seems like more tests that we have to write. I know
that most of the time the arguments will be valid because they are
sent in by other layers that have also been tested. Some times the
next layer above won't do anything with the arguments at all except
pass them to the next layer down.

So, the question is, do we keep writing checks at every level or do we
use a different approach? Is there a standard for this type of thing
where you work?

Some times I feel like throwing argument checks in at all layers since
I know that they cost relatively nothing. This would at least keep
things consistent at all layers of the system. Plus, each layer being
so reusable, it might be the "safest" alternative. However, the
thought of writing all those tests and all that code to perform a
check I can almost 100% of the time guaruntee will never be hit is
hard to stomach.

Personally, I favor doing argument checks at all levels. Partly it's a
legacy of COM programming for me, where you had to validate all the
time to respond the corresponding HRESULT. Partly it's because those
checks are really just a clumsy way of defining the method
precondition - and if you think of them as applied Design by Contract,
then clearly all public members at least should have them, otherwise
your contracts are simply not complete - and therefore meaningless.

Speaking of which, a library-based DbC solution is coming in .NET 4.0.
Have a look:

http://geekswithblogs.net/sdorman/archive/2008/11/07/clr-4.0-code-contracts..aspx

This is likely to be a solution far superior to manual "if (...) throw
ArgumentException" checks for a number of reasons (for example, it
supports full-fledged contract inheritance). And with it, there's
hardly any excuse not to spell out at least the preconditions of all
exposed members

That said, sometimes you do want to avoid repeated checks for
performance reasons. Sometimes the inliner can optimize them away,
sometimes not. More often than not it doesn't really matter either way
(the cost of you using a couple of lambdas in a method is likely to be
significantly higher, for example), but in API with lots of small
methods that's called a lot this can matter. If you look at the
publicly available .NET 3.5 BCL source code, you'll see that a lot of
places there don't do argument checks on internal & private members,
and this is usually documented as performance optimization.

 

[jehugaleahsa]

Personally, I favor doing argument checks at all levels. Partly it's a
legacy of COM programming for me, where you had to validate all the
time to respond the corresponding HRESULT. Partly it's because those
checks are really just a clumsy way of defining the method
precondition - and if you think of them as applied Design by Contract,
then clearly all public members at least should have them, otherwise
your contracts are simply not complete - and therefore meaningless.

Speaking of which, a library-based DbC solution is coming in .NET 4.0.
Have a look:

http://geekswithblogs.net/sdorman/archive/2008/11/07/clr-4.0-code-con...

This is likely to be a solution far superior to manual "if (...) throw
ArgumentException" checks for a number of reasons (for example, it
supports full-fledged contract inheritance). And with it, there's
hardly any excuse not to spell out at least the preconditions of all
exposed members

That said, sometimes you do want to avoid repeated checks for
performance reasons. Sometimes the inliner can optimize them away,
sometimes not. More often than not it doesn't really matter either way
(the cost of you using a couple of lambdas in a method is likely to be
significantly higher, for example), but in API with lots of small
methods that's called a lot this can matter. If you look at the
publicly available .NET 3.5 BCL source code, you'll see that a lot of
places there don't do argument checks on internal & private members,
and this is usually documented as performance optimization.- Hide quoted text -

- Show quoted text -

I like the idea of built-in code contracts. I am hoping that the code
contract class allows you to provide your own custom message to
describe a pre- or post-condition that failed. I am also hoping they
can come up with a slightly more reader-friendly way of presenting it.

CodeContract.Requires(CodeContract.ForAll(cart, s => s != i)) starts
to get a little too complex for non-developers. I guess the usefulness
of such a construct depends whether you are providing them for other
developers or for reviewers. It would be nicer to say, CodeContract:
Ensure i not in cart. -or- Ensure !cart.Contains(i). A slightly more
english syntax could go a long way.

The problem with s => s != i is that it requires an understanding of
delegates. Not everyone knows that s, here, represents each item in
the cart, one at a time. Try showing that to a manager or a junior
programmer some time! Prior to lambdas, this would be even more
difficult.

 

[Pavel Minaev]

I like the idea of built-in code contracts. I am hoping that the code
contract class allows you to provide your own custom message to
describe a pre- or post-condition that failed.

I'm not so sure about that, but I don't think you really need it,
either. Contract (in its code form) is already public - and the best
way to report it is to say that there is a contract violation (on
precondition, postcondition, invariant - wherever), and what the
expression was; much like a good assert (given that asserts are
primitive contracts, that's not surprising).

I am also hoping they
can come up with a slightly more reader-friendly way of presenting it.

CodeContract.Requires(CodeContract.ForAll(cart, s => s != i)) starts
to get a little too complex for non-developers. I guess the usefulness
of such a construct depends whether you are providing them for other
developers or for reviewers. It would be nicer to say, CodeContract:
Ensure i not in cart. -or- Ensure !cart.Contains(i). A slightly more
english syntax could go a long way.

The reason why there are new special methods, and why you can't just
use Enumerable.Contains() and Enumerable.Any/All is so that the static
verifier can follow the logic.

The problem with s => s != i is that it requires an understanding of
delegates. Not everyone knows that s, here, represents each item in
the cart, one at a time. Try showing that to a manager or a junior
programmer some time! Prior to lambdas, this would be even more
difficult.

Well, that's the best that can be done with a library solution. Note
that C# compiler is not aware of those - all the checks are inserted
by a post-compile IL rewriter. I do hope we'll get some syntactic
sugar in the language for that eventually, hopefully in C# 5...

That said, I don't think that lambdas are that hard. C# devs, even
junior ones, already have to learn them to deal with LINQ today, and
in my experience, if properly explained, it's something that's fairly
easy to grasp.