views:

85

answers:

1

I have the following class that implements IXmlSerializable. When implementing WriteXml(), I need to handle the case where the string members of this class may be null values. What is the best way of handling this?

Currently, I am using the default constructor in which all the string properties are initialized to empty string values. This way, when WriteXml() is called, the string will not be null.

One other way I could do this is check using String.IsNullOrEmpty before writing each string in xml.

Any suggestions on how I can improve this code?

    public sealed class FaxSender : IXmlSerializable
    {
        #region Public Constants

        private const string DEFAULT_CLASS_NAME = "FaxSender";

        #endregion Public Constants

        #region Public Properties

        public string Name { get; set; }

        public string Organization { get; set; }

        public string PhoneNumber { get; set; }

        public string FaxNumber { get; set; }

        public string EmailAddress { get; set; }

        #endregion Public Properties

        #region Public Methods

        #region Constructors

        public FaxSender()
        {
            Name = String.Empty;
            Organization = String.Empty;
            PhoneNumber = String.Empty;
            FaxNumber = String.Empty;
            EmailAddress = String.Empty;
        }

        public FaxSender(
            string name, 
            string organization, 
            string phoneNumber, 
            string faxNumber, 
            string emailAddress)
        {
            Name = name;
            Organization = organization;
            PhoneNumber = phoneNumber;
            FaxNumber = faxNumber;
            EmailAddress = emailAddress;
        }

        #endregion Constructors

        #region IXmlSerializable Members

        public System.Xml.Schema.XmlSchema GetSchema()
        {
            throw new NotImplementedException();
        }

        public void ReadXml(System.Xml.XmlReader reader)
        {
            throw new NotImplementedException();
        }

        public void WriteXml(System.Xml.XmlWriter xmlWriter)
        {
            try
            {
                // <sender>
                xmlWriter.WriteStartElement("sender");

                // Write the name of the sender as an element.
                xmlWriter.WriteElementString(
                    "name", 
                    this.Name.ToString(CultureInfo.CurrentCulture));

                // Write the organization of the sender as an element.
                xmlWriter.WriteElementString(
                    "organization", 
                    this.Organization.ToString(CultureInfo.CurrentCulture));

                // Write the phone number of the sender as an element.
                xmlWriter.WriteElementString(
                    "phone_number", 
                    this.PhoneNumber.ToString(CultureInfo.CurrentCulture));

                // Write the fax number of the sender as an element.
                xmlWriter.WriteElementString(
                    "fax_number", 
                    this.FaxNumber.ToString(CultureInfo.CurrentCulture));

                // Write the email address of the sender as an element.
                xmlWriter.WriteElementString(
                    "email_address", 
                    this.EmailAddress.ToString(CultureInfo.CurrentCulture));

                // </sender>
                xmlWriter.WriteEndElement();

            }
            catch
            {
                // Rethrow any exceptions.
                throw;
            }
        }

        #endregion IXmlSerializable Members

        #endregion Public Methods
    }
+1  A: 

I have some suggestions:

1 - Only implement IXmlSerializable as a last resort and instead control XML serialization through attributes. It's simpler and easier to maintain. It also handles the properties being a null reference.

[XmlRoot("sender")]
public sealed class FaxSender
{
    [XmlElement("name")]
    public string Name { get; set; }
    [XmlElement("organization")]
    public string Organization { get; set; }
    [XmlElement("phone_number")]
    public string PhoneNumber { get; set; }
    [XmlElement("fax_number")]
    public string FaxNumber { get; set; }
    [XmlElement("email_address")]
    public string EmailAddress { get; set; }

    // Remaining code omitted
}

2 - Do not wrap code in a try\catch if the only thing you are going to do is rethrow every exception;

3 - The guideline for constants in .NET is to use PascalCase which would result in DefaultClassName instead of DEFAULT_CLASS_NAME.

João Angelo