XML serialization helper class

Question

Yesterday was JSON serialization, today is XML serialization.

I've taken some of the suggestions from there and made modifications, as well as making other modifications that were not suggested but should be made.

    /// <summary>
    /// Provides methods for Serialization and Deserialization of XML/Extensible Markup Language documents.
    /// </summary>
    public class XmlSerialization
    {
        /// <summary>
        /// Serializes an object to an XML/Extensible Markup Language string.
        /// </summary>
        /// <typeparam name="T">The type of the object to serialize.</typeparam>
        /// <param name="value">The object to serialize.</param>
        /// <param name="serializedXml">Filled with a string that is the XmlSerialized object.</param>
        /// <param name="throwExceptions">If true, will throw exceptions. Otherwise, returns false on failures.</param>
        /// <returns>A boolean value indicating success.</returns>
        public static bool Serialize<T>(T value, ref string serializedXml, bool throwExceptions = false)
        {
#if DEBUG
#warning When in DEBUG Mode XML Serialization Exceptions will be thrown regardless of throwExceptions paramter.
            throwExceptions = true;
#endif

            if (value == null)
                if (throwExceptions)
                    throw new ArgumentNullException("The value is expected to be a non-null object.");
                else
                    return false;

            try
            {
                XmlSerializer xmlserializer = new XmlSerializer(typeof(T));

                using (StringWriter stringWriter = new StringWriter())
                using (XmlWriter writer = XmlWriter.Create(stringWriter))
                {
                    xmlserializer.Serialize(writer, value);

                    serializedXml = stringWriter.ToString();

                    return true;
                }
            }
            catch
            {
                if (throwExceptions)
                    throw;

                return false;
            }
        }

        /// <summary>
        /// Deserializes an XML/Extensible Markup Language string to an object.
        /// </summary>
        /// <typeparam name="T">The type of the object to serialize.</typeparam>
        /// <param name="value">The XML string representing the serialized object.</param>
        /// <param name="deserializedObject">Filled with the object that is the XmlDeserialized string.</param>
        /// <param name="throwExceptions">If true, will throw exceptions. Otherwise, returns false on failures.</param>
        /// <returns>A boolean value indicating success.</returns>
        public static bool Deserialize<T>(string value, ref T deserializedObject, bool throwExceptions = false)
        {
#if DEBUG
#warning When in DEBUG Mode XML Deserialization Exceptions will be thrown regardless of throwExceptions paramter.
            throwExceptions = true;
#endif

            if (value == null)
                if (throwExceptions)
                    throw new ArgumentNullException("The value is expected to be a non-null object.");
                else
                    return false;

            try
            {
                XmlSerializer xmlserializer = new XmlSerializer(typeof(T));

                using (StringReader stringReader = new StringReader(value))
                using (XmlReader reader = XmlReader.Create(stringReader))
                {
                    if (!xmlserializer.CanDeserialize(reader))
                            if (throwExceptions)
                                throw new ArgumentException("The provided value cannot be deserialized to the specified type.");
                            else
                                return false;

                    deserializedObject = (T)(xmlserializer.Deserialize(reader));

                    return true;
                }
            }
            catch
            {
                if (throwExceptions)
                    throw;

                return false;
            }
        }
    }

The idea is to keep it as simple as possible, and provide a common, natural form of accessing serialization.

A couple of changes:

• I added the exception that is thrown when value is null. This wasn't there until Heslacher mentioned that I only had one spot that exceptions were thrown, as I had forgot to put it on that check as well.
• throwErrors and all related messages/comments/documentation have been refactored to throwExceptions, errors to exceptions, et al.

Answer 1

                if (!xmlserializer.CanDeserialize(reader))
                        if (throwExceptions)
                            throw new ArgumentException("The provided value cannot be deserialized to the specified type.");
                        else
                            return false;  

I did tell you about braces {} didn't I ? Ok, I see you don't want to use them so don't whine if something goes terribly wrong.

So leaving that aside, lets talk about this snippet of code.

Because this is enclosed in a try..catch block which does check for throwExceptions there is no need to do it twice.

This can be simplified to (with braces ;-) )

if (!xmlserializer.CanDeserialize(reader))
{
    throw new ArgumentException("The provided value cannot be deserialized to the specified type.");
}   

You are using using statments for objects which implements IDisposable which is the preferred way

There isn't anything more to say but this methods are well structured, readable and also maintainable.

Answer 2

1. Your documentation comments are overly verbose and largely redundant in my opinion.

   • "Provides methods" - it pretty much goes without saying that it "provides methods", that's what classes are for (way more often than not).

   • "XML/Extensible Markup Language" - it's safe to assume that reader knows what XML is. And if they don't, expanding the acronym isn't likely to help.

   • "<returns>A boolean value" - I can see that it returns a boolean value. Why skip the information for throwExceptions? The comment currently sums up: "If true, will throw exceptions...". By the same logic it should rather read: "A boolean value which, if equal to true..." ;)

2. I'm not sure if it's such a hot idea to overwrite a parameter in debug mode (with your #if DEBUG directive), even if false was passed explicitly. It's sort of overdoing it, and probably at odds with the principle of least astonishment.

3. You know what type that is, so you could include both value.GetType() as well as typeof(T) in the exception message to make it more informative.

   throw new ArgumentException("The provided value cannot be deserialized to the specified type.");
   

4. (Some) documentation comments for your Deserialize method still talk about serializing instead of deserializing (copy-pasta).

5. Why make these methods static? Now it can't be subclassed, you can't create an interface out of it, you can't use dependency injection to pass this serializer around, you can't mock it. It will be tight coupling wherever you use it. It's falling back to the days of procedural programming. Note that one can always create singletons or static wrappers around a normal object if they need it.

Answer 3

Use braces for non-trivial statements inside conditional or loop

if (value == null)
{
    if (throwExceptions)
        throw new ArgumentNullException("The value is expected to be a non-null object.");
    else
        return false;
}

I am fine with no-braces around return, break, continue and throw. Alternative would be:

if (value == null)
{
    if (throwExceptions)
        throw new ArgumentNullException("The value is expected to be a non-null object.");
    return false;
}

as the else is unnecessary

Reuse code, don't duplicate

You can use one try...catch and have one if (throwExceptions) in the final catch for each method body. That could possibly have some speed penalty, but not something really important. Simplicity, readability and maintainability is more important (especially for managed language/code).

Why the bool argument?

Do you really need it? Does it have to be parameter? Are you calling it with variable or always true, false or default?

I would personaly create methods returning void that throws (e.g. void Serialize... and one with name starting with Try (e.g. bool TrySerialize...) that would call the first and return false on exception.

Actually, you intent to throw for DEBUG, to know it, to diagnose. So, it seems to be designed to throw, not to return false (this sounds like: we rather remain silent in release instead of crashing the app).