Repeated parameter checks in functions

Question

I often have call hierarchies in that all methods need the same parameters. If I dont't want to put them on the instance level (member of the class) then I ask me always if its meaningfull to check the validity of them in each method.

For example:

public void MethodA(object o){
   if(null == o){
      throw new ArgumentNullException("o");
   }
   // Do some thing unrelated to o

   MethodB(o);

   // Do some thing unrelated to o

}

public void MethodB(object o){
   if(null == o){
      throw new ArgumentNullException("o");
   }
   // Do something with o
}

If Method A uses the parameter, then its clear, I have to check the validity there and also in MethdoB. But as long as MethodA does nothing more with o than give it to MethodB, is it good practice to check the validity also in MethodA.

The avantage of checking also in MethodA may be that the exception throws in the method the callee has called, that is nice, but is it necessary? The call stack will state this also. Maybe its meaningfull in public,internal,protected but not in private methods?

I took the null-check as an example, but also index-validations or range validations fall in the self question, however I think there are limitations because of the danger of redundant code. What do you think?

UPDATE

Through the answer of AakashM I have seen that I was to little precise. MethodA not only calls MethodB, it does also other things but not related to o. I added an example to clarify this. Thanks AakashM.

Answer 1

I think you should ensure it once where the input value is produced (responsibility of the caller). This works if the class is used only by internal clients. If it's part of the public API, then you can't excape checks, but you might want to factorise them, as suggested in other answers.

For testing purposes, you can also use Debug.Assert(o != null) in both methods and the checks will be removed from release code, if performance is an issue.

Answer 2

Steve McConnell's Code Complete talks about the concept of 'the barricade' - a wall of defensive-ness outside of which data is untrusted, and inside of which data is trusted. Data that wants to enter the barricade must go through a validation process, but within the barricade, data is free to move around untrammelled by validation code.

If you can impose this amount of structuring and layering in your project, and stick to it, it does make inside-the-barricade code have less ceremony and more essence. But it only takes one method to call over the barricade for everything to go wrong.

In your example, MethodB is public. This means that you have no automatic future guarantees that MethodA will be its only caller - I would say therefore that its validation code should remain. If it were private to the class, however, you could make an argument for removing it.

As for MethodA, if it actually does nothing more than call MethodB, it shouldn't exist. If it's a stub for future expansion, and at some point it is going to do something with o, then its validation code should also remain.

Answer 3

Make a method for checking parameters and for each method where that is necessary put a private overload which has a bool parameter for checking parameters (using the method mentioned before).

The original method always calls the overload with check parameters bool as true.

public void MethodA(object o){
    checkArguments(o);

    methodB(o, false);

    // Do some thing unrelated to o
}

public void MethodB(object o){
    methodB(o, true);
}

private void methodB(object o, bool check)
{
    if (check) checkArguments(o);
    // Do things from original MethodB()

}

ADD: Since the parameter is always the same you could also use a delegate as one parameter of the check arguments method so you wouldn't need any overloads.

ADD2: Using design by contract approach you can leave the checking of the parameters as responsibility of the caller.

Answer 4

I don't consider parameter verification "redundant code". You need to protect all of the external entry points. If you have a private method such that you know by the time flow gets to it that all of its parameters are going to be valid then you perhaps have a case, but for public (and even protected) methods you really don't.