CODEWITHSUNDEEP
c#exception
adrianbanks
adrianbanks

Reputation: 82944

How should I store data inside custom exceptions?

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)?

Upvotes: 6

Views: 1530

Answers (4)

Matt Howells
Matt Howells

Reputation: 41266

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());
}

Upvotes: 2

Richie Cotton
Richie Cotton

Reputation: 121067

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.

Upvotes: 5

Skurmedel
Skurmedel

Reputation: 22149

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.

Upvotes: 1

Romain Verdier
Romain Verdier

Reputation: 12971

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.

Upvotes: 0

Related Questions