Best practices when validating function arguments for public/private methods [closed]

Question

Doing some research, it seems that people generally agree that arguments to public methods should be validated while private functions typically do not. This causes me to have some questions, but I have not been able to find a satisfactory answer so far.

Example:

public void DoSomething(int i)
{
    if (i < 0)
        throw new ArgumentOutOfRangeException("i");

    double d = DoWork(i);
}

private double DoWork(int i)
{
    double ret = ...; // some calculation
    return ret;
}

Thoughts:

1. What if the requirement of i to be non-negative changes inside DoWork()? The design risks leaving outdated validation checks in place. The programmer is responsible for adjusting the usage of a function that has changed, I know, but it leaves me wondering if there is a better way to minimize risk of error.

2. What about different calls to DoWork() not from DoSomething()? Must we validate the arguments redundantly?

public void DoSomething(int i)
{
    if (i < 0)
        throw new ArgumentOutOfRangeException("i");

    double d = DoWork(i);
}

public void DoSomethingElse()
{
    int i = 5;
    if (i < 0)
        throw new ArgumentOutOfRangeException("i");

    double d = DoWork(i);
}

private double DoWork(int i)
{
    double ret = ...; // some calculation
    return ret;
}

This can be cleaned up a little by putting the check into a function of its own. Then there is the risk that a new function making a call to DoWork(int i) will forget to validate i.

public void DoSomething(int i)
{
    ThrowIfIntegerIsNegative(i);

    double d = DoWork(i);
}

public void DoSomethingElse()
{
    int i = 5;
    ThrowIfIntegerIsNegative(i);

    double d = DoWork(i);
}

static void ThrowIfIntegerIsNegative(int i)
{
    if (i < 0)
        throw new ArgumentOutOfRangeException("i");
}

private double DoWork(int i)
{
    double ret = ...; // some calculation
    return ret;
}

Is that at all better than this?

public void DoSomething(int i)
{
    double d = DoWork(i);
}

public void DoSomethingElse()
{
    double d = DoWork(5);
}

private double DoWork(int i)
{
    if (i < 0)
        throw new ArgumentOutOfRangeException("i");

    double ret = ...; // some calculation
    return ret;
}

Depending on the situation, these are some goals I am trying to achieve simultaneously:

1. Have the argument validation in a single place (and possibly inside the function using the argument)
2. Report an error sooner rather than later (might not want to have a bunch of code run for an hour just to fail at the end for some bad user input)
3. Avoid validating arguments multiple times
4. Avoid performance impact in release code

How do you strike a balance? What methodology has worked best for you? I would greatly appreciate any insight.

Answer 1

The logic behind validating arguments in public methods and not validating arguments in private ones goes roughly as follows:

• When a public method is called with invalid arguments, it is a programming error outside your control.
• When a non-public method is called with invalid arguments, it is a logical error within your control.

This logic is reasonable: there is no need to waste cycles on validating the arguments to methods that your module produces internally. Private methods, on the other hand, could always assume that their arguments are valid, because you are in control of all calls of private methods.

However, it is very beneficial to catch situations when these assumptions are violated. To that end, it is a very good idea to use run-time assertions in place of argument validation inside private methods. This catches invalid invocations from outside callers with exceptions, and alerts you to invalid invocations from your own methods with asserts.