XmlCmdBuilder - doing away with inline xml

Question

I have to work with a 3rd-party API that allows me to define and execute "commands", using XML. Since I don't like seeing mixed abstraction levels, I managed to remove all the inline XML / string concatenations by creating a simple XmlCmdBuilder object - here is the C# implementation (I have one in VB6 as well, the C# one will eventually replace the VB6 equivalent code):

public class XmlCmdBuilder
{
    private readonly IList<XmlCmd> _xmlCommands;

    public XmlCmdBuilder()
    {
        _xmlCommands = new List<XmlCmd>();
    }

    public void AddCommand(XmlCmd cmd)
    {
        _xmlCommands.Add(cmd);
    }

    public void Clear()
    {
        _xmlCommands.Clear();
    }

    public override string ToString()
    {
        var builder = new StringBuilder();

        builder.Append("<cmd:Commands xmlns:cmd=\"http://www.contoso.com/XmlCommand\">");
        foreach (var xmlCommand in _xmlCommands)
        {
            builder.Append(xmlCommand);
        }
        builder.Append("</cmd:Commands>");

        return builder.ToString();
    }

}

The "builder" simply exposes a AddCommand method that takes in a XmlCmd object:

public class XmlCmd
{
    private readonly IList<XmlCmdParameter> _parameters;

    public XmlCmd(string name)
        : this(name, new List<XmlCmdParameter>())
    {

    }

    public XmlCmd(string name, params XmlCmdParameter[] parameters)
        : this(name, parameters.ToList())
    {

    }

    public XmlCmd(string name, IList<XmlCmdParameter> parameters)
    {
        _parameters = parameters;
        Name = name;
    }

    public string Name { get; private set; }

    public void AddParameter(XmlCmdParameter parameter)
    {
        _parameters.Add(parameter);
    }

    public override string ToString()
    {
        var builder = new StringBuilder();

        builder.Append(string.Format("<cmd:Command name=\"{0}\">", Name));
        foreach (var xmlCommandParameter in _parameters)
        {
            builder.Append(xmlCommandParameter);
        }
        builder.Append("</cmd:Command>");

        return builder.ToString();
    }
}

Similarly, the XmlCmd exposes a method to add XmlCmdParameter objects, which in turn expose methods to add XmlCmdParameterListItem objects.

This all works very well, but since most commands are predefined (there can be "custom" commands too), I created a factory class that creates XmlCmd objects off parameter values, like this:

public XmlCmd SetOptionValue(string optionName, string optionValue)
{
    var parameters = new[]
        {
            _parameterFactory.Create("name", optionName, XmlCmdParameterType.StringParam),
            _parameterFactory.Create("value", optionValue, XmlCmdParameterType.StringParam)
        };

    return new XmlCmd("SetOptionValue", parameters);
}

Then in the calling code, I can eliminate all the inline XML and string concatenation and replace it all with a single method call:

var builder = new XmlCmdBuilder();
builder.AddCommand(_cmdFactory.SetOptionValue("OptionName", "OptionValue"));

Then I can continue "building" the xml by adding subsequent AddCommand calls, and when I need to, I just call ToString() and pass the generated XML string to the 3rd-party API that knows what to do with it.

---

I have put these objects in their own class library, which needs a strong name key and ends up deployed in the GAC / Global Assembly Cache, so I want this library to be as "immune to change" as possible; one potential issue is with the XmlCmdFactory class: the commands are predefined, but I haven't implemented all of them and I might need to change it (add more methods) in the future.

I'd like some feedback on the XmlCmdBuilder and XmlCmd classes (XmlCmdParameter is built the same way, listing it here, along with the rest of the classes, would be almost redundant), and also some pointers in terms of extensibility, not so much about the code itself, but about adding more factory methods: something doesn't feel right about deploying a new version of this library to the GAC whenever a new XML command is needed.

Answer 1

I think there is almost never any need to write inline XML. Instead, you should use XElements and related types to represent XML. It will make your code simpler and safer (have you considered injection attacks?).

Another point is that it would be nice if your objects could be easily created using the object initializer syntax. For that, you need to implement IEnumerable and have method called Add(), but both are natural for your types.

Together, it would look something like:

public class XmlCmdBuilder : IEnumerable<XmlCmd>
{
    public static readonly XNamespace Namespace = "http://www.contoso.com/XmlCommand";

    private readonly IList<XmlCmd> _xmlCommands = new List<XmlCmd>();

    public XmlCmdBuilder()
    {
        _xmlCommands = new List<XmlCmd>();
    }

    public void Add(XmlCmd cmd)
    {
        _xmlCommands.Add(cmd);
    }

    public void Clear()
    {
        _xmlCommands.Clear();
    }

    public XElement GetXml()
    {
        return new XElement(
            Namespace + "Commands",
            new XAttribute(XNamespace.Xmlns + "cmd", Namespace),
            _xmlCommands.Select(cmd => cmd.GetXml()));
    }

    public IEnumerator<XmlCmd> GetEnumerator()
    {
        return _xmlCommands.GetEnumerator();
    }

    IEnumerator IEnumerable.GetEnumerator()
    {
        return GetEnumerator();
    }
}

public class XmlCmd : IEnumerable<XmlCmdParameter>
{
    public static readonly XNamespace Namespace = XmlCmdBuilder.Namespace;

    private readonly IList<XmlCmdParameter> _parameters;

    public XmlCmd(string name)
        : this(name, new List<XmlCmdParameter>())
    {}

    public XmlCmd(string name, params XmlCmdParameter[] parameters)
        : this(name, parameters.ToList())
    {}

    public XmlCmd(string name, IList<XmlCmdParameter> parameters)
    {
        _parameters = parameters;
        Name = name;
    }

    public string Name { get; private set; }

    public void Add(XmlCmdParameter parameter)
    {
        _parameters.Add(parameter);
    }

    public XElement GetXml()
    {
        return new XElement(
            Namespace + "Command",
            new XAttribute("name", Name),
            _parameters.Select(p => p.GetXml()));
    }

    public IEnumerator<XmlCmdParameter> GetEnumerator()
    {
        return _parameters.GetEnumerator();
    }

    IEnumerator IEnumerable.GetEnumerator()
    {
        return GetEnumerator();
    }
}

Usage could then look like this:

var builder = new XmlCmdBuilder
{
    new XmlCmd("cmd1") { new XmlCmdParameter(), new XmlCmdParameter() },
    new XmlCmd("cmd2")
};

Console.WriteLine(builder.GetXml());

Which outputs exactly what you'd expect:

<cmd:Commands xmlns:cmd="http://www.contoso.com/XmlCommand">
  <cmd:Command name="cmd1">
    <cmd:Parameter />
    <cmd:Parameter />
  </cmd:Command>
  <cmd:Command name="cmd2" />
</cmd:Commands>

Answer 2

You have to provide interfaces API, which helps users of your framework to add their custom interface implementations, and you have to replace default .ToString() ovverrides for readability and compatibility. In common, all is just fine.

Generally, you can provide any of the containers for user project extensibility (System.AddIn, for example) by using attributes, as a bonus, you can then control the versioning of you assembiles in GAC, and gain the ability to mark interfaces, classes and methods as [Obsolete] or [Deprecated] attributes

Also, if you provide an inteface-only assemblies with no of minimum dependencies of other project assembiles, you will keep in mind loose-coupling in design, which is good. Additinally, you can create reference interface implementations as a reference to developers in the future which are always work as a whatchdogs in deprecated classes.

public interface IXmlCmdBuilder
{
    string Xml { get; }
}

public interface IXmlCmd
{
    string Xml { get; }
}

public interface IXmlCmdParameter
{
    string Xml { get; }
}

as it listed here:

using System;
using System.Collections.Generic;
using System.Drawing;
using System.Linq;
using System.Net;
using System.Net.Sockets;
using System.Text;
using System.Threading.Tasks;

namespace Server
{
    public interface IXmlCmdBuilder
    {
        string Xml { get; }
    }

    public interface IXmlCmd
    {
        string Xml { get; }
    }

    public interface IXmlCmdParameter
    {
        string Xml { get; }
    }

    public sealed class XmlCmdBuilder : IXmlCmdBuilder
    {
        private readonly IList<IXmlCmd> _xmlCommands;

        public XmlCmdBuilder()
        {
            _xmlCommands = new List<IXmlCmd>();
        }

        public void AddCommand(IXmlCmd cmd)
        {
            _xmlCommands.Add(cmd);
        }

        public void Clear()
        {
            _xmlCommands.Clear();
        }

        public override string ToString()
        {
            return Xml;
        }

        public string Xml
        {
            get
            {
                var builder = new StringBuilder();

                builder.Append("<cmd:Commands xmlns:cmd=\"http://www.contoso.com/Xml\">");
                foreach (var xmlCommand in _xmlCommands)
                {
                    builder.Append(xmlCommand.Xml);
                }
                builder.Append("</cmd:Commands>");

                return builder.ToString();
            }
        }
    }

    public sealed class XmlCmd : IXmlCmd
    {
        private readonly IList<IXmlCmdParameter> _parameters;

        public XmlCmd(string name)
            : this(name, new List<IXmlCmdParameter>())
        {

        }

        public XmlCmd(string name, params IXmlCmdParameter[] parameters)
            : this(name, parameters.ToList())
        {

        }

        public XmlCmd(string name, IList<IXmlCmdParameter> parameters)
        {
            _parameters = parameters;
            Name = name;
        }

        public string Name { get; private set; }

        public void AddParameter(IXmlCmdParameter parameter)
        {
            _parameters.Add(parameter);
        }

        public class XmlCmdParameter : IXmlCmdParameter
        {
            public string Xml { get; private set; }

            public override string ToString()
            {
                return Xml;
            }
        }

        public override string ToString()
        {
            return Xml;
        }

        public string Xml
        {
            get
            {
                var builder = new StringBuilder();

                builder.Append(string.Format("<cmd:Command name=\"{0}\">", Name));
                foreach (var xmlCommandParameter in _parameters)
                {
                    builder.Append(xmlCommandParameter.Xml);
                }
                builder.Append("</cmd:Command>");

                return builder.ToString();
            }
        }
    }

    internal class Program
    {
        private static void Main(string[] args)
        {
            XmlCmdBuilder builder = new XmlCmdBuilder();
            builder.AddCommand(new XmlCmd(""));
            Console.WriteLine(builder.Xml);
        }
    }
}

Answer 3

something doesn't feel right about deploying a new version of this library to the GAC whenever a new XML command is needed

XmlCmd has public constructors, so you don't need factory methods: people can construct new XmlCmd without new factory methods in the GAC.

And/or perhaps you can add new factory methods to the GAC without changing the previous/existing factory class in the GAC, by coding the new factory methods as "extension methods" of your XmlCmdFactory.

Also you may be able to devise a XmlCmd constructor which makes it easier for users to create their own new factory methods; for example, something like this:

public XmlCmd(string[] names, params object[] parameterValues)
{
    this.Name = names[0]; // command name is first, parameter names are next
    assert(names.Length - 1 == parameterValues.Length);
    for (int i = 0; i < parameterValues.Length; ++i)
        _parameters.Add(CreateParameter(names[i + 1], parameterValues[i]));
}
private XmlCmdParameter CreateParameter(string name, object value)
{
    if (value is string)
    {
        return _parameterFactory.Create(pname, value, XmlCmdParameterType.StringParam)
    }
    ... etc. for other parameter types
}

Which you could call from a factory method like this:

public XmlCmd SetOptionValue(string optionName, string optionValue)
{
    string[] names = new string[] { "SetOptionValue", "name", "value" };
    return new XmlCmd(names, optionName, optionValue);
}

An advantage of this would be that it would hide (inside the XmlCmd constructor) the XmlCmdParameterType and whatever complexity there may be in converting non-string values (e.g. DateTime or whatever) to an XML string.

Answer 4

not sure exactly how this will be used, so this might be a null point.

you have add methods but you don't have any remove methods. not sure it would make extension easier, but it would probably provide you with a way to remove deprecated commands (you may or may not want to do this because of backwards compatibility in the future. but hey let someone else worry about that).

Let's say that you implement this in a GUI (in the future - sans aliens), you would want a remove method so that your Interface could show you what you have created so far and you could remove something that is an error.

maybe even a replace method for replacing a command that is broken again I am thinking of an interactive interface.

I always think of user usability, so the interface would allow you to create the XML by giving parameters, strings, etc.

maybe I am going off on a bunny trail, but I think it would be nice to have those methods so that you don't have to add them later.