views:

79

answers:

5

I am writing a class that does operations to multiple streams. Here is a example of what I am doing now

Dictionary<int, int> dict = new Dictionary<int, int>(_Streams.Count);
for (int i = 0; i < _Streams.Count; i++)
{
    try
    {
        dict.Add(i, _Streams[i].Read(buffer, offset, count));
    }
    catch (System.IO.IOException e)
    {
        throw new System.IO.IOException(String.Format("I/O exception occurred in stream {0}", i), e);
    }
    catch (System.NotSupportedException e)
    {
        throw new System.NotSupportedException(String.Format("The reading of the stream {0} is not supported", i), e);
    }
    catch (System.ObjectDisposedException e)
    {
        throw new System.ObjectDisposedException(String.Format("Stream {0} is Disposed", i), e);
    }
}
int? last = null;
foreach (var i in dict)
{
    if (last == null)
        last = i.Value;
    if (last != i.Value)
        throw new ReadStreamsDiffrentExecption(dict);
    last = i.Value;
}
return (int)last;

I would like to simplify my code down to

Dictionary<int, int> dict = new Dictionary<int, int>(_Streams.Count);
for (int i = 0; i < _Streams.Count; i++)
{
    try
    {
        dict.Add(i, _Streams[i].Read(buffer, offset, count));
    }
    catch (Exception e)
    {
        throw new Exception(String.Format("Exception occurred in stream {0}", i), e);
    }
}
int? last = null;
foreach (var i in dict)
{
    if (last == null)
        last = i.Value;
    if (last != i.Value)
        throw new ReadStreamsDiffrentExecption(dict);
    last = i.Value;
}
return (int)last;

However if anyone is trying to catch specific exceptions my wrapper will hide the exception that Read threw. How can I preserve the type of exception, add my extra info, but not need to write a handler for every possible contingency in the try block.

+1  A: 

I think you have a bit of an issue here in the way you are working with your exception.

  1. You should not be throwing the base Exception class, but something more specific so they can handle it.
  2. Is the id value something that is really "valuable" from a diagnostic function?

I would review what you are doing, and see if you really need to be wrapping the exception.

Mitchel Sellers
+1  A: 

I find the first version is better for readability and is the more expressive to my eye. This is how exception handling should be written.

Oded
+3  A: 

I would suggest not catching those exceptions at all...

The information you add could (mostly) be gleaned from the stackdump.

You could use catch-and-wrap to translate to a library-specific exception:

 catch (Exception e)
 {
    throw new ReadStreamsErrorExecption(
      String.Format("Exception occurred in stream {0}", i), e);
 }
Henk Holterman
The reason I am trying to avoid relying on the stack-dump is for the next revision I plan on threading the read and write operations.
Scott Chamberlain
Except doesn't the code you propose discard the original stack trace?
JeffH
@JeffH, I proposed 2 ways, the first is: don't touch. In the secnd you would have 2 stacktraces to puzzle with.
Henk Holterman
@Scott: Threading doesn't change the problem that much.
Henk Holterman
+1  A: 

Generally the rule that I've picked up from Eric Lipperts blogs, is that you should only capture an exception if you're going to do something about it.

Here you are just re-throwing the exception with a new message. Just let the client handle the exceptions themselves unless you're going to try and recover from errors. In which case add a

throw;

If you need to bubble the exception backup because you can't handle it.

Ian
Ian, It is sometimes productive to 'change' an exception, that is why the Exception class has the InnerException property. Doing so is not really a 'handling' it, and `throw;` does not apply.
Henk Holterman
@Henk : noted, thank you :)
Ian
+1  A: 

One little known .NET trick is that you CAN add information to an Exception without wrapping it. Every exception has a .Data dictionary on it that you can stuff with additional information, e.g.

try
{
   ...
}
catch (FileNotFoundException ex)
{
   ex.Data.Add("filename", filename);
   throw;
}

Now in your top-level exception handling code you can dump the exception and its associated dictionary out to your log file or into your Exceptions database and thus get far more information than you had before.

In an ASP.NET application you might want to add the URL, the username, the referrer, the contents of the cookies, ... to the .Data dictionary before letting your application error handler take it.

Hightechrider