10

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
Preet Sangha
  • 64,563
  • 18
  • 145
  • 216

5 Answers5

8

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.

Jeff Mercado
  • 129,526
  • 32
  • 251
  • 272
6

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.

Community
  • 1
  • 1
Kevin
  • 8,312
  • 4
  • 27
  • 31
5

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.

jbtule
  • 31,383
  • 12
  • 95
  • 128
  • That's an interesting use of `dynamic`. I keep forgetting that it is one of our options. – Jeff Mercado Aug 19 '11 at 19:51
  • Most dynamic languages don't offer method overloading, so even when you are used to it, you tend to think double dispatch which is much more involved. It's one of the ntat side effects of C# being a true hybrid static/dynamic language now. – jbtule Aug 19 '11 at 22:01
5
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).

svick
  • 236,525
  • 50
  • 385
  • 514
  • Are the concerns really separate? Sure in the specified cases the `int` and the `string` case share the same GetValue code, but when we are talking about the classification and serialization of data should those ever be separate, would the reverse be ever be true (ie something type would share the same GetTypeName method but have a different GetValue)? No...never. – jbtule Aug 19 '11 at 00:50
  • The only reason I can find for the difference is that the different type pertains to the SQL type this object ultimately used for. But yes fair point. – Preet Sangha Aug 19 '11 at 01:17
  • 1
    @Preet, I meant that svick mentioned a separation of concerns in regards to GetTypeName & GetValue, and my point is that they aren't separate concerns and you should have GetTypeNameAndValue as one method. It's really only a fluke that a GetValue implementation is shared between different GetTypeName implementations, and you'd never have a GetTypeName implementation shared between different GetValue implementations, thus separating them into two methods really is a smell and if more types get added to the equation it is clear. It is the only substantial difference svick made from my answer. – jbtule Aug 19 '11 at 01:51
  • @jbtule, I primarily meant separating the `ConvertToXml()` and the other methods. I don't think there is much difference between your approach and mine. (Also, I didn't read your answer before posting, I probably wouldn't even answer if you were faster.) – svick Aug 19 '11 at 18:47
  • @svick Faster? It was 20 minutes--not 20 seconds. Anyway, aside from omitting the xml header and spliting name and value, the only other difference is your use of `DateTime?` instead of `DateTime` which as @Jeff-Mercado made clear in his answer is a redundancy. Nullables can only be used statically, once it is cast to an `object`(or `dynamic`) the clr strips it down to just the value and you lose the nullable type info. In both the reference implementation and yours, a `DateTime` is being converted to a `DateTime?` just to pull out the original `DateTime` value again. – jbtule Aug 19 '11 at 22:22
2

Reapproached considering new requirements.

  • Decoupled transformation details of each concrete type and XML generation logic itself
  • Easily could be introduced new data type support by adding a new factory to the provider. Currently supported types set is limited by the TypeCode enumeration members but obviously this can be easily switched to an other type selector/identifier.
  • I must agree with jbtule that Tuple.Create() really looks much better rather then construction of KeyValuePair<,>, never used it before, nice stuff, thanks!

Method itself:

public string ConvertToXml(
    IDictionary<string, object> rawData, 
        Dictionary<TypeCode, Func<object, Tuple<string, string>>> transformationFactoryProvider) 
{
    XmlDocument doc = new XmlDocument();
    doc.LoadXml("<?xml version='1.0' encoding='utf-8'?><sc/>");

    if (rawData != null)
    {
        Func<object, Tuple<string, string>> defaultFactory = 
              (raw) => Tuple.Create("string", raw.ToString());

        foreach (KeyValuePair<string, object> item in rawData)
        {
            TypeCode parameterTypeCode = Type.GetTypeCode(item.Value.GetType());
            var transformationFactory = transformationFactoryProvider.ContainsKey(parameterTypeCode)
                                            ? transformationFactoryProvider[parameterTypeCode]
                                            : defaultFactory;

            var transformedItem = transformationFactory(item.Value);
            XmlElement xmlElement = doc.CreateElement("pr");
            xmlElement.SetAttribute("tp", transformedItem.Item1);
            xmlElement.SetAttribute("nm", item.Key);
            xmlElement.SetAttribute("vl", transformedItem.Item2);
            doc.FirstChild.NextSibling.AppendChild(xmlElement);
        }
    }

    return doc.OuterXml; 
}

How-to use example:

// Transformation Factories
// Input: raw object
// Output: Item1: type name, Item2: value in the finally formatted string
Func<object, Tuple<string, string>> numericFactory = raw => Tuple.Create("int", raw.ToString());
Func<object, Tuple<string, string>> dateTimeFactory =
    raw => Tuple.Create("datetime", (raw as DateTime?).GetValueOrDefault().ToString("o"));

// Transformation Factory Provider
// Input: TypeCode
// Output: transformation factory for the given type
var transformationFactoryProvider =
    new Dictionary<TypeCode, Func<object, Tuple<string, string>>>
        {                        
            {TypeCode.Int16, numericFactory},
            {TypeCode.Int32, numericFactory},
            {TypeCode.Int64, numericFactory},
            {TypeCode.DateTime, dateTimeFactory}
        };

// Convert to XML given parameters
IDictionary<string, object> parameters = new Dictionary<string, object>
                         {
                                { "SOMEDATA", 12 },
                                { "INTXX", 23 },
                                { "DTTM", DateTime.Now },
                                { "PLAINTEXT", "Plain Text" },
                                { "RAWOBJECT", new object() },
                          };
string xmlParameters = this.ConvertToXml(parameters, transformationFactoryProvider);
sll
  • 61,540
  • 22
  • 104
  • 156
  • 1
    Oh come on, your KeyValuePairs would look much better as Tuples. `Tuple.Create` vs `new KeyValuePair` just saying. – jbtule Aug 19 '11 at 01:06