views:

163

answers:

4

When dealing with custom exceptions, I usually inherit from Exception and then add some fields/properties to my exception class to store some additional info:

public class MyException : Exception
{
    public int ErrorCode{get;set;}

    public MyException()
    {}
}

In the above example, the ErrorCode value is stored in the exception, meaning that I have to add it to and retireve if from the SerializationInfo object in the protected constructor and the overridden GetObjectData method.

The Exception class has a Data property, which according to MSDN:

Gets a collection of key/value pairs that provide additional user-defined information about the exception.

If I store the error code inside the Data, it will get serialised for me by the Exception class (according to Reflector), meaning that my exception class now looks like:

public class MyException : Exception
{
    public int ErrorCode
    {
        get {return (int) Data["ErrorCode"];}
        set {Data["ErrorCode"] = value;}
    }

    public MyException()
    {}
}

This means that whilst there is a bit more work to do in dealing with the get/set of the error code (like dealing with casting errors and situations where the error code might not be in the dictionary), I don't have to worry about serialising/deserialising it.

Is this just two different ways of achieving the same thing, or does one way have any clear advantage(s) over the other (apart from those I've already mentioned)?

A: 

You should go with the first solution. I can't see much value in the Data property, unless you plan to raise row Exception instances with attached infos.

If you have your own Exception type, then use properties instead: it's more clean and safe.

Romain Verdier
+1  A: 

Microsoft's own guidelines:

Do make exceptions serializable. An exception must be serializable to work correctly across application domain and remoting boundaries.

I would store it in the Data-property which sadly would let outside code modify the value without consent, or use solution 1 (in your example) but make it serializeable. In the end I would probably go for solution 1 so I can be sure that the value never changes.

Skurmedel
I was planning on making it serializable which ever way I implemented it.
adrianbanks
+4  A: 

If you are bothering to create your own exception, you don't need the Data property. Data comes in useful when you want to store a bit of extra information in an existing exception class, but don't want to create your own custom exception class.

Richie Cotton
+1  A: 

I would avoid using Data as it is not under your control e.g. some code somewhere might decide to overwrite the "ErrorCode" value. Instead use the propery and implement serialization. I use the following code to test all my custom exceptions to make sure I've implemented them properly.

public static void TestCustomException<T>() where T : Exception
{
    var t = typeof(T);

    //Custom exceptions should have the following 3 constructors
    var e1 = (T)Activator.CreateInstance(t, null);

    const string message = "message";
    var e2 = (T)Activator.CreateInstance(t, message);
    Assert.AreEqual(message, e2.Message);

    var innerEx = new Exception("inner Exception");
    var e3 = (T)Activator.CreateInstance(t, message, innerEx);
    Assert.AreEqual(message, e3.Message);
    Assert.AreEqual(innerEx, e3.InnerException);

    //They should also be serializable
    var stream = new MemoryStream();
    var formatter = new BinaryFormatter();
    formatter.Serialize(stream, e3);
    stream.Flush();
    stream.Position = 0;
    var e4 = (T)formatter.Deserialize(stream);
    Assert.AreEqual(message, e4.Message);
    Assert.AreEqual(innerEx.ToString(), e4.InnerException.ToString());
}
Matt Howells