Recommendations for checking function arguments for public / private methods

Question

By doing some research, it seems that people generally agree that the arguments of public methods should be checked, and private functions should not. This causes me some questions, but so far I have not been able to find a satisfactory answer.

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:

• What if requirement i non-negative changes inside DoWork() ? The project risks leaving outdated verification checks in place. The programmer is responsible for setting up the use of a function that has changed, I know, but I'm not interested in whether there is a better way to minimize the risk of error.

• What about different calls to DoWork() not from DoSomething() ? Should we check 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 cleared up a bit by putting a check in its own function. Then there is a risk that a new function that calls DoWork(int i) will forget to confirm 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 it really better?

 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 of the goals that I am trying to achieve at the same time:

• Test the arguments in one place (and possibly inside the function using the argument)
• Report the error sooner rather than later (you probably don’t want the heap of code to run for an hour, just to not work at the end for incorrect user input)
• Avoid checking arguments multiple times
• Avoid Performance Impacts in Release Code

How do you make a balance? Which methodology is best for you? I would really appreciate your understanding.

Answer 1

The logic of checking arguments in public methods and not checking arguments in special cases looks something like this:

• When a public method is called with invalid arguments, this is a programming error beyond your control.
• When calling a non-public method with invalid arguments, this is a logical error in your control.

This logic is reasonable: there is no need to spend cycles checking arguments for the methods that your module produces internally. Private methods, on the other hand, can always assume that their arguments are valid because you control all calls to private methods.

However, it is very important to catch situations where these assumptions are violated. To this end, it is a very good idea to use runtime statements instead of validation within private methods. This raises invalid calls to external callers with exceptions and alerts you to invalid calls from your own methods with claims.

Answer 2

I like checking with exceptions in public methods:

 public void Foo(int i) { if (i < 0) throw new ArgumentOutOfRangeException("i"); double d = DoWork(i); } 

And I like to check with Asserts in private methods:

 private double DoWork(int i) { System.Diagnostics.Debug.Assert(i >= 0); // .... } 

• This means re-checking in several publicly available methods.
• It detects invalid parameters as early as possible.
• Throwing an ArgumentOutOfRangeException exception due to an invalid parameter inside the private method can confuse the caller of the public method due to differences in parameter names, etc.
• Often your public methods do extra work before invoking private methods to save work.
• Use unit testing to ensure that new methods do not miss any validation checks.

Another thing to do is handle any protected method, such as a public method.

Edit:

This is an example of a bad check that violates # 3 above:

 public void Foo(int distanceInMetres) { double d = DoWork(distanceInMetres * 1000); } private double DoWork(int distanceInMillimetres) { if (distanceInMillimetres < 0) throw new ArgumentOutOfRangeException("distanceInMillimetres"); // .... } 

If the calling Foo object sees an exception in the parameter "distanceInMillimetres", it will be confused because it called "Foo", which accepted a parameter called "distanceInMetres".

Answer 3

Accept dasblinkenlight's claims about public, not public, methods.

If you practice TDD, your private methods are the result of code refactoring and splitting some public methods into private methods. Therefore, if you test public methods using tests, your private methods will be automatically tested.

If you do not use TDD, but want to check that the logic is not broken, you can use different methods, for example, it claims ( http://msdn.microsoft.com/en-us/library/ttcc4x86.aspx ) or code contracts ( http : //msdn.microsoft.com/en-us/library/dd264808.aspx ).