0

Here's an example. I saw a "ReadOnlyDictionary" class online and it had the following code:

void ICollection.CopyTo(Array array, int index)
{
    ICollection collection = new List<KeyValuePair<TKey, TValue>>(this._source);

    collection.CopyTo(array, index);
}

For example, should I check array for a null argument, or should I let the the CopyTo method do that for me? It just seems a bit redundent, but if best practices say to check everything in your own method, then that's what I want to do. I'm just not sure what "best practices" says to do.

michael
  • 14,844
  • 28
  • 89
  • 177

2 Answers2

1

I think it wise to say if you plan to do something with array that relies on it NOT being null then you should check this. But if it just a pass through then I don't see a reason why you should check.

Another thought is if the method gets complicated in the future. You might still want to check for it because someone may modify the code and use array without realizing that it might be null. This is only for maintaining good code in my opinion.

Amir Raminfar
  • 33,777
  • 7
  • 93
  • 123
1

If somebody else's library or API* is going to complain about my inputs, I don't want to give it those inputs, I want to validate and/or complain first. This is especially important if calls into external APIs are expensive, such as a database or web service call.

You know what inputs the API is going to reject. Don't send those, invalidate them in your own public API.


*Note: I consider my own public boundaries to be the same thing. If I have class Foo that does not like given arguments, if I invoke Foo, at some level before doing so, I'm going to validate my arguments. You don't do this at every level (assume there are layers of indirection, maybe, private methods calling into private methods, etc.), but at some reasonable public boundary, I will validate. Validate early, don't let complicated logic or work be done when it's just going to be rejected anyway.

Anthony Pegram
  • 123,721
  • 27
  • 225
  • 246