Should I throw my own ArgumentOutOfRangeException or let one bubble up from below?

You should throw your own, for a couple of reasons.

You should throw your own, for a couple of reasons: You can explicitly set the appropriate parameter name in the constructor. This way, the exception has the appropriate parameter information for the Argument that is out of range. (Minor) The internal list's exception will have an invalid stack trace as far as your user is concerned.By constructing a new exception, you can get the stack trace showing your method as being the one that's inappropriate, which will be easier for your user's debugging.

As for catching and modifying the internal exception - I wouldn't recommend this, necessarily. If the exception is one where the extra information would potentially be useful, you should use the InnerException of your new exception to propogate this information up.In this example (ArgumentOutOfRangeException), the fact that there is an internal list should be kept as an implementation detail, and there's no reason for your user to see that information.

I decided that IF I do throw my own, or rethrow the lower one, I will at least add param name (though in this method there is only one so it's sort of a given which param is guilty) – Neil N Mar 26 '10 at 20:20 @Neil: I just edited to answer that question, too. – Reed Copsey Mar 26 '10 at 20:21 @Neil: There is no way to "add" the param to an existing exception. The List will have one included, but I wouldn't rely on it staying the same... That being said, I would NEVER modify an exception in a handler... – Reed Copsey Mar 26 '10 at 20:29 @Reed: I suppose you could argue that it's cleaner to do your own range check and throw your own exception.

However, in the sample code, the variable name is passed unchanged and the fact that a List is used is not particularly dangerous to share. Consider cases where you are implementing an indexer and the underlying collection is a Dictionary; you'd have to search twice, first with Contains, then with the Indexer. That seems like a lot of overhead and work for very little gain.

– Steven Sudit Mar 26 '10 at 20:49 @Steven: You're assuming that List's indexer uses a parameter name of "index" (remember, it's up to the List implementation to handle this, and can change on you without breaking API compat...) You're kind of proving my point, though - if you decided to change from a List to some other collection, letting the exception bubble up would change your API's exception behavior. That should be a complete implementation detail... – Reed Copsey Mar 26 '10 at 21:46.

...breaks my OOP goal of the user not needing to know about the underlying objects. Throwing the same exception does nothing for encapsulation. Either your (implied/documented - since we don't have checked exceptions or DbC) contract states that you'll throw an ArgumentOutOfRange exception in this case or not.

How that exception is generated, and what it's stack trace looks like is irrelevant to a caller. If you move your internal implementation to something that doesn't throw an ArgumentOutOfRange exception, then you'll need to throw one yourself to fulfill your contract (or do a breaking change to the contract). Stack traces (and the param name) is for the guy debugging - not for programmatic access.

Unless there's some security concern, there's no worry to letting them "leak". BTW, the advice (which you may be thinking of) of wrapping exceptions comes from a more abstract scenario. Consider an IAuthenticationService that throws if a user can't Login, If there's both an LdapAuthenticationService and a DatabaseAuthenticationService implementation, then you'd have to catch both LdapDirectoryException and SqlException to determine a failed login.

When a 3rd implementation is done, you'd need to add it's specific exception types as well. By all implementers wrapping their specific exception in a FailedAuthenticationException, you only have the single type to worry about. It'd still be a good idea to include the original exception in InnerException though, since it aids in debugging.

I hope you can see the difference between this scenario and yours - in your case, you're just throwing the same exception type so there's no difference. All that being said, if it was for a library - then I'd check for it and throw. But not for OOP purity - but because it makes the contract explicit and less likely to be changed inadvertently.

If I was using T|BDD, then I'd just write a test for it and let the List throw instead - the test makes the contract explicit.

If you're on . NET 2.0 or above, you can use the inner exception, like so: public RenderedImageInfo GetValue(int index) { try { listindex. LastRetrieved = DateTime.

Now; return listindex; } catch (ArgumentOutOfRangeException ex) { throw new ArgumentOutOfRangeException("index", ex); } } That way, your user will primarily see your exception, but it is possible to dig deeper if you (or they) want to. Edit: I see now that InnerException has been supported since . NET 1.0, it's just that constructor that is new.

I see no way to actually set the InnerException for an ArgumentOutOfRangeException in . NET before 2.0. Did they just forget it, or is the code I wrote above against the intended use?

1 for covering all the bases. – Adam Maras Mar 26 '10 at 20:30 I think this is what I was getting at when I said add/modify the param name in the exception. But overall I think Reed's answer of just throwing a new one is better.

+1 for completeness though. – Neil N Mar 26 '10 at 20:36 This isn't a good general solution. What do you do when you need to check one List and then another?

– Steven Sudit Mar 26 '10 at 20:52 To be clear, it could be made more general-purpose by replacing "index" with a variable that is set to different values as the code progresses, such that it's index1 before we use index1, and so on. Is that clearer? – Steven Sudit Mar 26 '10 at 17:26.

It's fine to let the exception bubble up in this case. Edit I've been asked to update my answer to include some of the responses... Arguably, catching the exception and rethrowing it as an inner is more robust, in that it insulates you from changes and creates a more consistent interface. Having said that, the benefit can be very small -- bordering on none -- in an environment where the developer has access to all the code.

Moreover, whatever small benefit we obtain is not free. The additional error-handling code adds complexity, and must itself be tested to ensure that it works. It also adds to the risk of breaking the method during maintenance, in that the basic functionality of the method could be obscured by all of the bullet-proofing.So, is it worth it?

It depends. It comes down to cost/benefit analysis, which can only be done for a specific context. For a public API, such as AlphaFS, the benefits may well exceed the costs.

For internal use on code that is not designed for significant reuse, it probably isn't. That's the context of my original answer, which I still stand by. Also, as ghtechrider pointed out, there may be other technologies, such as coding contracts, which yield much the same benefit but at a lower cost, changing the equation in favor of robustness.

As always, I am open to reasoned disagreement. However, I suggest that we all try to avoid one-size-fits-all thinking. Edit Just to make it clear that I'm not crazy when I say that error-handling code is error-prone, take a look at the original question and tell me if you can spot the bug.

I offer a +1 if you do.

Reed made some interesting points about why there are times you want to throw your own exception, but it would be helpful if the downvoter were to explain why it's not fine IN THIS CASE. – Steven Sudit Mar 26 '10 at 20:46 I didn't downvote, but I think its sort of ironic that you ask for a reason from a downvoter, when your answer didn't have any reasoning behind it. WHY is it fine in this case?

– Neil N Mar 26 '10 at 22:25 1 Well, if the downvoter had asked me WHY, I would have had an opportunity to answer. As it is, I've answered through my responses to others. The summary is that there's no benefit but there's a cost.

Either way, the same exception gets thrown, differing only in the depth of its stack trace. Why complicate code for no reason? – Steven Sudit Mar 27 '10 at 0:18 +1."Why complicate code for no reason" - I agree.

Throw ArgumentNullException rather than dereferencing a null reference, to avoid a NullReferenceException being thrown. But I wouldn't bother if passing the reference to a lower level library. – Joe Mar 28 '10 at 18:25 1 I would update your answer to explain that simplicity is good: more code usually means more errors and makes it harder to actually see what a method does.

If this class is for internal consumption it's probably just fine. If this is . NET 4, a Code Contract would be better here because it will help document what the method expects and may enable compile time checking of the arguments.

– ghtechrider May 7 '10 at 16:05.

I cant really gove you an answer,but what I can give you is a way to a solution, that is you have to find the anglde that you relate to or peaks your interest. A good paper is one that people get drawn into because it reaches them ln some way.As for me WW11 to me, I think of the holocaust and the effect it had on the survivors, their families and those who stood by and did nothing until it was too late.