What is the best way to refactor code with many if and duplicated logic for extracting a value from different Controls classes

Question

I have the following code in the WPF application

if (panel != null)
{
     IList listOfValues = new ComparableListOfObjects();
        var childControls = panel.GetChildren<Control>(x => x.Visibility == Visibility.Visible);
 foreach (Control childControl in childControls)
 {
    var textBox = childControl as TextBox;
    if (textBox != null)
    {
        listOfValues.Add(textBox.Text);
        continue;
    }

    var comboBox = childControl as ComboBox;
    if (comboBox != null)
    {
        listOfValues.Add(comboBox.SelectedItem);
        continue;
    }

    var datePicker = childControl as DatePicker;
    if (datePicker != null)
    {
        listOfValues.Add(datePicker.SelectedDate.GetValueOrDefault());
        continue;
    }
    var numericBox = childControl as NumericUpDown;
    if (numericBox != null)
    {
        listOfValues.Add(numericBox.Value);
        continue;
    }

}

What is the best approach to refactor this code with repetition the same logic for extract value from different controls like?

        var numericBox = childControl as NumericUpDown;
    if (numericBox != null)
    {
        listOfValues.Add(numericBox.Value);
        continue;
    }

In the same class in other method there is the same code

        private static object GetControlValue(Control control)
    {
        if (control == null)
            throw new ArgumentNullException("control");

        var textBox = control as TextBox;
        if (textBox != null)
            return textBox.Text;

        var comboBox = control as ComboBox;
        if (comboBox != null)
            return comboBox.SelectedValue;

        var datePicker = control as DatePicker;
        if (datePicker != null)
            return datePicker.SelectedDate.GetValueOrDefault();

        var numericUpDown = control as NumericUpDown;
        if (numericUpDown != null)
            return numericUpDown.Value;

        throw new NotSupportedException();
    }

May by I should use the strategy design pattern but in this case I need to create additional classes for each type of control?

Could you suggest me better solotion for this prolem? Thanks in advance.

Answer 1

Having if and switch statements are not a bad thing. Even doing some rudimentary type checking is not necessarily a bad thing, particularly when the types in use can't be used polymorphically. Having that logic expressed more than once is what is frowned upon, because you are repeating yourself, and you have multiple maintenance points for the same change.

In your original code snippet, you do

var blah = obj as Foo;
if (blah != null)
{
    someList.Add(blah.Value); 
}

And repeat this for several more control types. But then in your private method later, you have basically the same logic expressed the same number of times.

var blah = obj as Foo;
if (blah != null)
    return blah.Value;

The only difference is that in the first snippet, you take the value and add it to the list. In the second, you return the value. The first snippet should do away with its type-checking logic, it should use the logic already expressed in the other method.

foreach (var control in childControls)
{
    listOfValues.Add(GetControlValue(control));
}

The idea is don't repeat yourself. DRY.