Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

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

I have a class that wraps List<>

I have GetValue by index method:

    public RenderedImageInfo GetValue(int index)
    {
        list[index].LastRetrieved = DateTime.Now;
        return list[index];
    }

If the user requests an index that is out of range, this will throw an ArgumentOutOfRangeException .

Should I just let this happen or check for it and throw my own? i.e.

    public RenderedImageInfo GetValue(int index)
    {
        if (index >= list.Count)
        {
            throw new ArgumentOutOfRangeException("index");
        }
        list[index].LastRetrieved = DateTime.Now;
        return list[index];
    }

In the first scenario, the user would have an exception from the internal list, which breaks my OOP goal of the user not needing to know about the underlying objects.

But in the second scenario, I feel as though I am adding redundant code.

Edit:
And now that I think of it, what about a 3rd scenario, where I catch the internal exception, modify it, and rethrow it?

like image 422
Neil N Avatar asked Mar 26 '10 20:03

Neil N


2 Answers

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

  1. 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.
  2. (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.

like image 105
Reed Copsey Avatar answered Oct 25 '22 10:10

Reed Copsey


If you're on .NET 2.0 or above, you can use the inner exception, like so:

public RenderedImageInfo GetValue(int index)
{
    try {
        list[index].LastRetrieved = DateTime.Now;
        return list[index];
    } 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?

like image 30
Thomas Avatar answered Oct 25 '22 08:10

Thomas