Should I catch all possible specific exceptions or

问题:

Let's say I want to deserialize some XML file to a strongly typed object. In case this XML file can't be deserialized (for whatever reason) I would just create a default object and continue normal application workflow without showing any errors to the user. (actually this application is running as Windows service so there is no user for it... for example, imagine application trying to load config file and if fails then just use default config).

My questions is how to create Deserialize() method such that it is easy to use by calling code? My main concerns are how to handle exceptions... Here is my research.

Solution 1:

Here is the simplest way:

    public static T Deserialize<T>(string xml)
    {
        var serializer = new XmlSerializer(typeof (T));
        using (var sr = new StreamReader(xml))
        using (var reader = XmlReader.Create(sr))
            return (T) serializer.Deserialize(reader);
    }

I think usage of this method would be very hard in the calling code because I'd have to handle all possible exceptions that can be thrown by any of these methods/constructors. Calling code doesn't care about that, it only cares whether operation succeeded or not.

So here is my second try:

Solution 2:

    public static T Deserialize<T>(string xml)
    {
        try
        {
            var serializer = new XmlSerializer(typeof (T));
            using (var sr = new StreamReader(xml))
            using (var reader = XmlReader.Create(sr))
                return (T) serializer.Deserialize(reader);
        }
        catch (ArgumentException ex)
        {
            throw new XmlDeserializeException(ValidationExceptionMsg, ex);
        }
        catch (IOException ex)
        {
            throw new XmlDeserializeException(ValidationExceptionMsg, ex);
        }
        catch (XmlSchemaException ex)
        {
            throw new XmlDeserializeException(ValidationExceptionMsg, ex);
        }
        catch (InvalidOperationException ex)
        {
            throw new XmlDeserializeException(ValidationExceptionMsg, ex);
        }
    }

Here I have created a custom exception called XmlDeserializeException and use it to wrap all exceptions that can be thrown by methods (as specified in MSDN) in the try block. Now, the calling code should only catch XmlDeserializeException to know there was an error. However I am not sure how good is this solution... If I need to create a lot of methods like this then all of them would be having a lot of catch blocks that only wrap exception to a custom exception.

So I was wondering would following code is better:

Solution 3:

    public static T Deserialize<T>(string xml)
    {
        try
        {
            var serializer = new XmlSerializer(typeof (T));
            using (var sr = new StreamReader(xml))
            using (var reader = XmlReader.Create(sr))
                return (T) serializer.Deserialize(reader);
        }
        catch (Exception ex)
        {
            throw new XmlDeserializeException(ValidationExceptionMsg, ex);
        }
    }

Here I catch general Exception and wrap it in custom XmlDeserializeException. This way I have reduced code writing, there is no code redundancy and there is less clutter. The calling code will again only have to catch XmlDeserializeException like in solution 2.

Which solution should I use and why? Is there any better way? Please keep in mind the scenario where I want to use this Deserialize() method, this is not a library/framework but rather an application that doesn't have user interactivity.

回答1:

If you handle the exceptions the same way there really is no point in having different catches, in which case you keep clutter to a minimum and go with solution 3.

Solution 2 is just a whole lot more code to do the exact same thing.

回答2:

In your question you say:

Let's say I want to deserialize some XML file to a strongly typed object. In case this XML file can't be deserialized (for whatever reason) I would just create a default object and continue normal application workflow without showing any errors to the user.

If you do nothing with the exception (as you state, there is no user interface, just a window service), you could log your exception, and return a default(T) instead of propagating an exception, like this:

public static T Deserialize<T>(string xml)
    {
        try
        {
            var serializer = new XmlSerializer(typeof (T));
            using (var sr = new StreamReader(xml))
            using (var reader = XmlReader.Create(sr))
                return (T) serializer.Deserialize(reader);
        }
        catch (Exception ex)
        {
            // Log exception here:
            Logger.Log(...)
            return default(T);
        }
    }

回答3:

I assume your Deserialize operation is in a technical component that is called by a business component; and so I would keep it as clean and simple as possible:

public static T Deserialize<T>(string xml)
{
    try
    {
        var serializer = new XmlSerializer(typeof (T));
        using (var sr = new StreamReader(xml))
        using (var reader = XmlReader.Create(sr))
            return (T) serializer.Deserialize(reader);
    }
    catch (Exception)
    {
        // clean up if needed 
        throw; // throw and keep stack trace
    }

In your caller or a general error handling component, you catch the exeption; and act on it; for example, do logging (including stack trace), translate exception to business exception etc.

Note that:

• In your Deserialize operation you rethrow the exception, keeping stack trace
• In your Deserialize operation you throw an exception, don't return a status. If it is an exception, throw it, don't swallow it and rely on statuses.
• In your caller, handle the exception and do whatever you have to do: transform the exception to a fault in case of a WCF service, for example. There would be a different catch only if you have different behavior in handling the exception.