Optimizing large switch statement

Question

I have large switch statement in which I create UIElements based on input value from XElement:

public static UIElement CreateElement(XElement element) {
            var name = element.Attribute("Name").Value;
            var text = element.Attribute("Value").Value;
            var width = Convert.ToDouble(element.Attribute("Width").Value);
            var height = Convert.ToDouble(element.Attribute("Height").Value);
            //...
            switch (element.Attribute("Type").Value) {
                case "System.Windows.Forms.Label":
                    return new System.Windows.Controls.Label() {
                        Name = name,
                        Content = text,
                        Width = width,
                        Height = height
                    };
                case "System.Windows.Forms.Button":
                    return new System.Windows.Controls.Button() {
                        Name = name,
                        Content = text,
                        Width = width,
                        Height = height
                    };
                    //...
                default:
                    return null;
            }
        }

I am creating a lot controls like this and as you can see, too much repetition is going on.

Is there some way to avoid this repetition? Thanks in advance for ideas.

Flynn1179 · Accepted Answer · 2011-05-23T08:52:03.497

6

You could create a generic function that does the create:

private static Create<T>(string name, string text, double width, double height) where T: Control, new()
{
   return new T { Name = name, Content = text, Width = width, Height = height }
}

Your switch then becomes:

switch (element.Attribute("Type").Value) {
  case "System.Windows.Forms.Label" : return Create<System.Windows.Forms.Label>(name, text, width, height);
  etc.
}

You could also adapt this to pass in the XElement, whichever you prefer.

If the Type attribute is always the name of the System.Type you want, then you could just do

Control ctrl = (Control) Activator.CreateInstance(Type.GetType(element.Attribute("Type").Value));
ctrl.Name = name;
etc.

If there's a one to one mapping between the value of the attribute and the type you want, then you can declare a readonly static field with the mapping:

private static readonly uiTypeMapping = new Dictionary<string,Type> {
  { "System.Windows.Forms.Label", typeof(System.Windows.Controls.Label) },
  { "System.Windows.Forms.Button", typeof(System.Windows.Controls.Button) },
  { etc. }
};

And use

UIElement elem = (UIElement) Activator.CreateInstance(uiTypeMapping[element.Attribute("Type").Value]);
etc.

edited May 23 '11 at 08:52

answered May 23 '11 at 08:37

Flynn1179

11,925
6
38
74

Note that in the OP, a different type than the one in the "Type" text is created (the text is a Winforms label, but a WPF label is created, for example). So using Activator won't help. – Daniel Rose May 23 '11 at 08:46
Ah, so it is, never spotted that. I'll adapt my answer. – Flynn1179 May 23 '11 at 08:48
Note that this one does not solve the complexity problem. If you reformat the code, you will see that even the code size is not reduced. – George Polevoy May 23 '11 at 10:00
Not the first suggestion, no, although passing in the XElement probably would. Using Activator definitely will, although I have to be honest, I prefer @Allrameest's answer, it's probably more efficient. – Flynn1179 May 23 '11 at 10:06

score 6 · Answer 2 · answered May 23 '11 at 08:55

Something like this could work... :)

var controlCreators = new Dictionary<string, Func<ContentControl>>
                        {
                            {"System.Windows.Forms.Label", () => new Label()},
                            {"System.Windows.Forms.Button", () => new Button()}
                        };

Func<ContentControl> createControl;
if (!controlCreators.TryGetValue(element.Attribute("Type").Value, out createControl))
{
    return null;
}

var control = createControl();
control.Name = name;
control.Content = text;
control.Width = width;
control.Height = height;
return control;

Heh, I never thought of using functions in the dictionary. Good call. — Flynn1179, May 23 '11 at 09:19

Daniel Rose · Answer 3 · 2011-05-23T08:50:46.433

1

Those different controls have inheritance trees. So for example Width, Height, Name are defined on FrameworkElement. So you could do something like the following:

object createdObject = null;
switch (element.Attribute("Type").Value)
{
case "System.Windows.Forms.Label":
    createdObject = new System.Windows.Controls.Label();
    break;
case "System.Windows.Forms.Button":
    createdObject = new System.Windows.Controls.Button();
    break;
}

var fe = createdObject as FrameworkElement;
if (fe != null)
{
    fe.Name = element.Attribute("Name").Value;
    fe.Width = Convert.ToDouble(element.Attribute("Width").Value);
    fe.Height = Convert.ToDouble(element.Attribute("Height").Value);
}

var ce = createdObject as ContentElement;
if (ce != null)
{
     ce.Content = element.Attribute("Value").Value;
}

return createdObject;

Note that by using this approach, in comparison to Flynn's answer, you can also easily add code such as "when the control is an ItemsControl, do this", i.e. code which won't apply to every type, but only to some of them.

edited May 23 '11 at 08:50

answered May 23 '11 at 08:43

Daniel Rose

17,233
9
65
88

I think you need some `break`s to make it compile. – H H May 23 '11 at 08:50
Hm, are you sure? both [Button](http://msdn.microsoft.com/en-us/library/system.windows.controls.button.aspx) and [Label](http://msdn.microsoft.com/en-us/library/system.windows.controls.button.aspx) are contentcontrols, but he might have others in there ofcourse – aL3891 May 23 '11 at 08:53
Since we don't know what other logic there is, we can only speculate. It works for the given code. If for other ContentControls something different should be done, then setting the correct Content could be directly done in the `new XXX()` statements. – Daniel Rose May 23 '11 at 08:53
I can easily add code with Flynn's approach too. For example: 'if(T is ContentControl){ (ContentControl)T).Content = text }. I am already working on it. – Vale May 23 '11 at 09:31
Yes, there's nothing to stop you from combining the approaches. – Daniel Rose May 23 '11 at 10:50

score 1 · Answer 4 · answered May 23 '11 at 09:48

You can do it with reflection + expressions.

[TestClass]
public class UnitTest1
{
    public class Creator
    {
        private static Dictionary<string,Func<XElement, Control>> _map = new Dictionary<string, Func<XElement,Control>>();

        public static Control Create(XElement element)
        {
            var create = GetCreator(element.Attribute("Type").Value);

            return create(element);
        }

        private static Expression<Func<XElement, string>> CreateXmlAttributeAccessor(string elementName)
        {
            return (xl => xl.Attributes(elementName).Select(el => el.Value).FirstOrDefault() ?? "_" + elementName);
        }

        private static Func<XElement, Control> GetCreator(string typeName)
        {
            Func<XElement, Control> existing;
            if (_map.TryGetValue(typeName, out existing))
                return existing;

            // mapping for whatever property names you wish
            var propMapping = new[]
            {
                new{ Name = "Name", Getter = CreateXmlAttributeAccessor("Name") },
                new{ Name = "Content", Getter = CreateXmlAttributeAccessor("Value") },
            };

            var t = Assembly.GetAssembly(typeof (Control)).GetType("System.Windows.Controls." + typeName);

            var elementParameter = Expression.Parameter(typeof (XElement), "element");

            var p = from propItem in propMapping
                    let member = t.GetMember(propItem.Name)
                    where member.Length != 0
                    select (MemberBinding)Expression.Bind(member[0], Expression.Invoke(propItem.Getter, elementParameter));

            var expression = Expression.Lambda<Func<XElement, Control>>(
                Expression.MemberInit(Expression.New(t),p), elementParameter);

            existing = expression.Compile();
            _map[typeName] = existing;

            return existing;
        }
    }

    [TestMethod]
    public void TestMethod1()
    {
        var xel = new XElement("control",
            new XAttribute("Type", "Button"),
            new XAttribute("Name", "Foo"),
            new XAttribute("Value", "Bar"),
            new XElement("NonExistent", "foobar")); // To check stability

        var button = (Button) Creator.Create(xel);

        Assert.AreEqual("Foo", button.Name);
        Assert.AreEqual("Bar", button.Content);
    }
}

To make it work with other types then string, you can use Expression.Convert. Left as an exercise.

Interesting approach, but a little to hard for me. I will return to it when I have time to analyze it. Definitely useful answer. — Vale, May 23 '11 at 10:01
What is the advantage of using Reflection here? I only see that it complicates the code and that you lose compile-time type checking. — Daniel Rose, May 23 '11 at 10:49
Advantage is that cardinality of possible control types does not affect the algorithm. In other words, chances are that is all the code you need to write, no matter how many control types you need to serve. — George Polevoy, May 24 '11 at 10:24
Also note, it's not just a reflection. Please note `expression.Compile()`. When hitting same type twice, you get an already compiled delegate, so it's faster on large amount of controls. — George Polevoy, May 24 '11 at 10:27

score 0 · Answer 5 · answered May 23 '11 at 08:36

0

You could to this with reflection instead, or you could create a Dictionary of strings (what you're switching on now) and Funcs (or actions rather) where you create the controls.

for the specific code you posted, you can assign height and width after the switch statement, since they exsist on Control directly.

answered May 23 '11 at 08:36

aL3891

6,205
3
33
37

I got your idea, but can you write dummy code on how to use actions and dictionary, I don't know how to begin. Thanks – Vale May 23 '11 at 08:39
It's ok, Flynn gave the answer – Vale May 23 '11 at 08:40

Optimizing large switch statement

5 Answers5