Parametercheck in a method

Question

I have opended a question about repeated parameter-checks in public methods.

The result there seemed clear to me but out of this I have another question concerning these parameter checks (I had the same question in mind posting my last question but did the formulation awkward and wanted too much (more than one question in one post), I hope this time it’s better):

---

If I have repeating parameters that will be given to public Methods of a class of mine, the parameters must be validated in each public Method. If I delegate the check to another (private) method such as

void EnsureIndexIsValid(int index){
     if(index <0 || index > A_VALUE){
         throw new IndexOutOfRangeException(“..message..”);
     }
}

then the throwing method is EnsureIndexIsValidIndex and not the called method. Is this bad practice?

If its bad practice, one possibility would be to catch the exception in the using method and then re-throw the exception. Is this good practice (The source of the exception for the caller is now AMethod, not the EnsureIndexIsValid )? Or is it only overhead?

public void AMethod(int index,....){
  try{
      EnsureIndexIsValid(index)
  }catch(IndexOutOfRangeException e){
      throw e;
  }
  // ....

EDIT: The following seems definitivey not to be a good idea

If this would be good practice, would it be legitimate to catch a general exception for more of these checks and then re-throw:

public void AMethod(int index,....){
  try{
      EnsureIndexIsValid(index);
      EnsuresValueIsInValidRange(anotherParamter);
  }catch(Exception e){
      throw e;
  }
  // ...

Conclusion:

For all that come to this thread, here the short answer:

Delegation of parameter-checks seems to be a common pattern and is reasonable. However re-throwing such exceptions to hide the validation method is not clever. Better return a boolean value from the validation method and then throw the exception in the public method if the check fails.

Answer 1

You should not be catching Exception, just the ones you can deal with so you shouldn't use your last example.

Answer 2

No I don't think it's bad practice to delegate validation to another method, and I wouldn't re-throw exceptions from higher-level methods just to give that impression. The client doesn't really care where an exception comes from, just why it has been thrown.

Answer 3

Hi,

I'd say it's actually good practice to throw the exception in the validation method as that's the location where the problem occurs (the validation fails). And if you need to know who called the validation method the exception will provide that information in its callstack. So I would not catch and rethrow the validation exception.

HTH.

Answer 4

I would much rather have the private method be bool IndexIsValid, then the actual public method looks like

if(!IndexIsValid(parameterToThisMethod))
{
    throw new ArgumentOutOfRangeException("parametersToThisMethod");
}

This has the advantage of having the actual method at the top of the call stack.

Also, even if you did it the other way (with the private validator method throwing), this pattern:

catch(IndexOutOfRangeException e)
{
    throw e;
}

doesn't do nice things to the call-stack at all; either simply throw;, or throw new IndexOutOfRangeException("parameter", e);