Validating function arguments?

Question

On a regular basis, I validate my function arguments:

public static void Function(int i, string s)
{
  Debug.Assert(i > 0);
  Debug.Assert(s != null);
  Debug.Assert(s.length > 0);
}

Of course the checks are "valid" in the context of the function.

Is this common industry practice? What is common practice concerning function argument validation?

Answer 1

I won't speak to industry standards, but you could combine the bottom two asserts into a single line:

Debug.Assert(!String.IsNullOrEmpty(s));

Answer 2

I try not to use Debug.Assert, rather I write guards. If the function parameter is not of expected value, I exit the function. Like this:

public static void Function(int i, string s)
{
    if(i <= 0)
    {
        /*exit and warn calling code */
    }
}

I find this reduces the amount of wrangling that need to happen.

Answer 3

Most of the time I don't use Debug.Assert, I would do something like this.

public static void Function(int i, string s)
{
  if (i > 0 || !String.IsNullOrEmpty(s))
         Throw New ArgumentException("blah blah");
}

WARNING: This is air code, I havn't tested it.

Answer 4

You should not be using asserts to validate data in a live application. It is my understanding that asserts are meant to test whether the function is being used in the proper way. Or that the function is returning the proper value I.e. the value that you are getting is what you expected. They are used a lot in testing frameworks. They are meant to be turned off when the system is deployed as they are slow. If you would like to handle invalid cases, you should do so explicitly as the poster above mentioned.

Answer 5

Any code that is callable over the network or via inter process communication absolutely must have parameter validation because otherwise it's a security vulnerability - but you have to throw an exception Debug.Assert just will not do because it only checks debug builds.

Any code that other people on your team will use also should have parameter validations, just because it will help them know it's their bug when they pass you an invalid value, again you should throw exceptions this time because you can add a nice description ot an exception with explanation what they did wrong and how to fix it.

Debug.Assert in your function is just to help YOU debug, it's a nice first line of defense but it's not "real" validation.

Answer 6

The accepted practice is as follows if the values are not valid or will cause an exception later on:

if( i < 0 )
   throw new ArgumentOutOfRangeException("i", "parameter i must be greater than 0");

if( string.IsNullOrEmpty(s) )
   throw new ArgumentNullException("s","the paramater s needs to be set ...");

So the list of basic argument exceptions is as follows:

ArgumentException
ArgumentNullException
ArgumentOutOfRangeException

Answer 7

What you wrote are preconditions, and an essential element in Design by Contract. Google (or "StackOverflow":) for that term and you'll find quite a lot of good information about it, and some bad information, too. Note that the method includes also postconditions and the concept of class invariant.

Let's leave it clear that assertions are a valid mechanism.

Of course, they're usually (not always) not checked in Release mode, so this means that you have to test your code before releasing it.

If assertions are left enabled and an assertion is violated, the standard behaviour in some languages that use assertions (and in Eiffel in particular) is to throw an assertion violation exception.

Assertions left unchecked are not a convenient or advisable mechanism if you're publishing a code library, nor (obviously) a way to validate direct possibly incorrect input. If you have "possibly incorrect input" you have to design as part of the normal behaviour of your program an input validation layer; but you can still freely use assertions in the internal modules.

---

Other languages, like Java, have more of a tradition of explicitly checking arguments and throwing exceptions if they're wrong, mainly because these languages don't have a strong "assert" or "design by contract" tradition.

(It may seem strange to some, but I find the differences in tradition respectable, and not necessarily evil.)

See also: design by contract tests by assert or by exception?

Answer 8

For public functions, especially API calls, you should be throwing exceptions. Consumers would probably appreciate knowing that there was a bug in their code, and an exception is the guaranteed way of doing it.

For internal or private functions, Debug.Assert is fine (but not necessary, IMO). You won't be taking in unknown parameters, and your tests should catch any invalid values by expected output. But, sometimes, Debug.Assert will let you zero in on or prevent a bug that much quicker.

For public functions that are not API calls, or internal methods subject to other folks calling them, you can go either way. I generally prefer exceptions for public methods, and (usually) let internal methods do without exceptions. If an internal method is particularly prone to misuse, then an exception is warranted.

While you want to validate arguments, you don't want 4 levels of validation that you have to keep in sync (and pay the perf penalty for). So, validate at the external interface, and just trust that you and your co-workers are able to call functions appropriately and/or fix the bug that inevitably results.

Answer 9

You should use Assert to validate programmatic assumptions; that is, for the situation where

1. you're the only one calling that method
2. it should be an impossible state to get into

The Assert statements will allow you to double check that the impossible state is never reached. Use this where you would otherwise feel comfortable without validation.

For situations where the function is given bad arguments, but you can see that it's not impossible for it to receive those values (e.g. when someone else could call that code), you should throw exceptions (a la @Nathan W and @Robert Paulson) or fail gracefully (a la @Srdjan Pejic).