Challenge: Can you make this simple function more elegant using C# 4.0

Question

As I hack through our code base I just noticed this function. It converts an IDictionary<string, object> (Paramters - an instance variable) into an XML string.

This is nothing but curiosity on my part :-) .

So can it be written with a lot less code using C# 4.0?Rule: no external libs except the .Net Framework BCL.

To make it more of a challenge I'm not putting the input dictionary spec here, as you should be able to work it out from the code.

public string ConvertToXml() {
    XmlDocument doc = new XmlDocument();
    doc.LoadXml("<?xml version='1.0' encoding='utf-8'?><sc/>");
    foreach (KeyValuePair<string, object> param in Parameters) {
        XmlElement elm = doc.CreateElement("pr");

        if (param.Value is int || param.Value is Int32 || param.Value is Int16 || param.Value is Int64) {
            elm.SetAttribute("tp", "int");
        } else if (param.Value is DateTime?){
            elm.SetAttribute("tp", "datetime");
        } else {
            elm.SetAttribute("tp", "string");
        }

        elm.SetAttribute("nm", param.Key);
        if (param.Value is DateTime?) {
            DateTime? dateTime = param.Value as DateTime?;
            elm.SetAttribute("vl", dateTime.Value.ToString("o"));
        } else{
            elm.SetAttribute("vl", param.Value.ToString());
        }
        doc.FirstChild.NextSibling.AppendChild(elm);
    }
    return doc.OuterXml;
}

Let me add some more thoughts.

To me :

• less is more but terse is bad
• more types are fine, but trivial types seem smelly
• reusability is good

Answer 1

Using LINQ to XML can make this very simple to write up. Prefer this over the standard XML libraries if you have the choice.

I believe this should be equivalent:

public static string ToXmlString(this IDictionary<string, object> dict)
{
    var doc = new XDocument(new XElement("sc", dict.Select(ToXElement)));

    using (var writer = new Utf8StringWriter())
    {
        doc.Save(writer); // "hack" to force include the declaration
        return writer.ToString();
    }
}

class Utf8StringWriter : StringWriter
{
    public override Encoding Encoding { get { return Encoding.UTF8; } }
}

static XElement ToXElement(KeyValuePair<string, object> kvp)
{
    var value = kvp.Value ?? String.Empty;

    string typeString;
    string valueString;
    switch (Type.GetTypeCode(value.GetType()))
    {
    case TypeCode.Int16:
    case TypeCode.Int32:
    case TypeCode.Int64:
        typeString = "int";
        valueString = value.ToString();
        break;
    case TypeCode.DateTime:
        typeString = "datetime";
        valueString = ((DateTime)value).ToString("o");
        break;
    default:
        typeString = "string";
        valueString = value.ToString();
        break;
    }

    return new XElement("pr",
        new XAttribute("tp", typeString),
        new XAttribute("nm", kvp.Key),
        new XAttribute("vl", valueString));
}

Note that checking if the value is of type DateTime? is pointless. I'm not sure what value there is in storing null values in a dictionary but if it was possible, you'd lose that type information anyway by virtue of making values of type object.

Also, if there was a DateTime? value that wasn't null, then the value itself would be boxed, not the Nullable<DateTime> structure itself. So the actual type would be DateTime which is why this code works.

Answer 2

Using dynamic and LINQ to XML:

ConvertToXml can be reduced to one statement (assuming omitting the XML declaration is acceptable).

public string ConvertToXml()
{
    return new XElement("sc",
        Parameters.Select(param => CreateElement(param.Key, (dynamic)param.Value))
    ).ToString(SaveOptions.DisableFormatting);
}

Note that CreateElement casts param.Value to dynamic so that the correct overload from the following will be selected at runtime.

XElement CreateElement(string key, object value)
{
    return CreateElement("string", key, value.ToString());
}

XElement CreateElement(string key, long value)
{
    return CreateElement("int", key, value.ToString());
}

XElement CreateElement(string key, DateTime value)
{
    return CreateElement("datetime", key, value.ToString("o"));
}

The overloads above ultimately call:

XElement CreateElement(string typename, string key, string value)
{
    return new XElement("pr",
        new XAttribute("tp", typename),
        new XAttribute("nm", key),
        new XAttribute("vl", value)
    );
}

This code is reduces the number of statements (although not lines) found in the question. This approach builds on svick's, but reduces the number of methods and dynamic calls required.

Answer 3

Using .net 4.0 features such as Tuple and dynamic keyword. My test cases produce the precise output of the original question.

//using System;
//using System.Collections.Generic;
//using System.Linq;
//using System.Xml.Linq;

    public string ConvertToXml()
    {
        //Create XDocument to specification with linq-to-xml
        var doc = new XDocument(
            new XElement("sc",
                    from param in Parameters
                    //Uses dynamic invocation to use overload resolution at runtime
                    let attributeVals = AttributeValues((dynamic)param.Value)
                    select new XElement("pr",
                                new XAttribute("tp", attributeVals.Item1),
                                new XAttribute("nm", param.Key),
                                new XAttribute("vl", attributeVals.Item2)
                           )
            )
        );
        //Write to string
        using (var writer = new Utf8StringWriter())
        {
            doc.Save(writer, SaveOptions.DisableFormatting);//Don't add whitespace
            return writer.ToString();
        }
    }
    //C# overloading will choose `long` as the best pick for `short` and `int` types too
    static Tuple<string, string> AttributeValues(long value)
    {
        return Tuple.Create("int", value.ToString());
    }
    //Overload for DateTime
    static Tuple<string, string> AttributeValues(DateTime value)
    {
        return Tuple.Create("datetime", value.ToString("o"));
    }
    //Overload catch all
    static Tuple<string, string> AttributeValues(object value)
    {
        return Tuple.Create("string", value.ToString());
    }
    // Using John Skeet's Utf8StringWriter trick
    // http://stackoverflow.com/questions/3871738/force-xdocument-to-write-to-string-with-utf-8-encoding/3871822#3871822
    class Utf8StringWriter : System.IO.StringWriter
    {
        public override System.Text.Encoding Encoding { get { return System.Text.Encoding.UTF8; } }
    }

Optionally: Change let statement to:

let attributeVals = (Tuple<string,string>)AttributeValues((dynamic)param.Value)

That would limit dynamic invocation to just that line. But since there isn't much else going on I thought it would be cleaner looking to not add the additional cast.

Answer 4

public string ConvertToXml()
{
    var doc = new XDocument(
        new XElement("sd",
            Parameters.Select(param =>
                new XElement("pr",
                    new XAttribute("tp", GetTypeName((dynamic)param.Value)),
                    new XAttribute("nm", param.Key),
                    new XAttribute("vl", GetValue((dynamic)param.Value))
                    )
                )
            )
        );
    return doc.ToString();
}

This code assumes you have overloaded methods GetTypeName() and GetValue() implemented as:

static string GetTypeName(long value)
{
    return "int";
}

static string GetTypeName(DateTime? value)
{
    return "datetime";
}

static string GetTypeName(object value)
{
    return "string";
}

static string GetValue(DateTime? value)
{
    return value.Value.ToString("o");
}

static string GetValue(object value)
{
    return value.ToString();
}

This uses the fact that when using dynamic, the correct overload will be chosen at runtime.

You don't need overloads for int and short, because they can be converted to long (and such conversion is considered better than conversion to object). But this also means that types such as ushort and byte will get tp of int.

Also, the returned string doesn't contain the XML declaration, but it doesn't make sense anyway to declare that an UTF-16 encoded string is UTF-8 encoded. (If you want to save it in a UTF-8 encoded file later, returning and saving the XDocument would be better and would write the correct XML declaration.)

I think this is a good solution, because it nicely separates concerns into different methods (you could even put GetTypeName() and GetValue() overload into different class).