Geek Answers Handbook

Is there a case where checking parameters can be considered redundant?

The first thing I do in the public method is to check every single parameter before they get the opportunity to use, pass or reference, and then throw an exception if any of them break the contract. I found this a very good practice, as it allows you to catch the offender at the time of the violation, but then, quite often, I write a very simple getter / indexer, for example:

private List<Item> m_items = ...; public Item GetItemByIdx( int idx ) { if( (idx < 0) || (idx >= m_items.Count) ) { throw new ArgumentOutOfRangeException( "idx", "Invalid index" ); } return m_items[ idx ]; }

In this case, the index parameter directly relates to the indexes in the list, and I know that it is a fact (for example, documentation) that the list itself will do the same and will throw the same exception. Should I remove this check, or should I leave it alone?

I wanted to know what you guys think, because I am now in the middle of refactoring a large project, and I have found many cases, as shown above.

Thanks in advance.

+4
source share
3 answers

It's not just a matter of taste, consider

 if (!File.Exists(fileName)) throw new ArgumentException("..."); var s = File.OpenText(fileName);

This is similar to your example, but there are several reasons (concurrency, permissions) why the OpenText() method can still fail, even with a FileNotFound error. Thus, Exists-check simply gives a false sense of security and control.

This is a reasonable thing, when you write the GetItemByIdx method, it probably looks quite reasonable. But if you look around in a random piece of code, there are usually many assumptions that you could check before continuing. It is just not practical to check them all over again. We must be selective.

So, in a simple pass method like GetItemByIdx, I would mind the redundant checks. But as soon as a function adds more functionality or there is a very explicit specification that says something about idx, this argument is rotated.

As a rule, an exception should be chosen if a clearly defined condition is violated and that the condition is relevant at the current level. If the condition refers to a lower level, then let this level process it.

+6
source

I would only perform a parameter check where this would lead to some improvement in code behavior. Since you know that in this case the check will be performed by the list itself, then your own check will be redundant and will not give any additional value, so I would not worry.

+3
source

It is true that you may have duplicated work that has already been done in the API, but now it is. If your error handling infrastructure works and is robust and does not cause performance problems ( IYF profiling ), then I should leave it, and gradually phase if you have time. This does not seem to be the top priority!

+1
source

More articles:


All Articles