Каков наилучший способ рефакторинга кода со множеством логических элементов if и дубликатов для извлечения значения из различных классов Controls - PullRequest
4 голосов
/ 21 октября 2011

У меня есть следующий код в приложении WPF

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;
    }

}

Каков наилучший подход к рефакторингу этого кода с повторением одной и той же логики для извлечения значения из разных элементов управления, например?

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

В том же классе в другом методе есть тот же код

        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();
    }

Можно ли использовать шаблон проектирования стратегии, но в этом случае мне нужно создать дополнительные классы для каждого типа элемента управления?

Не могли бы вы порекомендовать мне лучшее посвящение этой проблеме? Заранее спасибо.

Ответы [ 5 ]

6 голосов
/ 21 октября 2011

Наличие if и switch утверждений - это не плохо.Даже выполнение некоторой элементарной проверки типов не обязательно является плохой вещью, особенно когда используемые типы не могут быть использованы полиморфно.Выражение этой логики более одного раза вызывает нарекания, потому что вы повторяете себя, и у вас есть несколько точек обслуживания для одного и того же изменения.

В исходном фрагменте кода вы делаете

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

И повторяете это для нескольких других типов элементов управления.Но потом в вашем частном методе вы в основном использовали ту же логику, выраженную столько же раз.

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

Единственное отличие состоит в том, что в первом фрагменте вы берете значение и добавляете его в список.Во втором вы возвращаете значение.Первый фрагмент должен убрать с его логикой проверки типов, он должен использовать логику, уже выраженную в другом методе.

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

Идея в том, что не повторяйся. СУХОЙ.

1 голос
/ 21 октября 2011

Вы можете сделать это как вариант выбора в универсальном методе, но с этим стилем еще есть работа:

public static string GetValue<T>(T obj) where T:Control
        {
            switch (obj.GetType().ToString())
            {
                case "TextBox":
                    return (obj as TextBox).Text;
                    break;
                case "ComboBox":
                    return (obj as ComboBox).SelectedValue.ToString();
                    break;

                    //..etc...
            }   
        }
1 голос
/ 21 октября 2011

Немного взлома, но вот способ использования делегатов и инициализаторов коллекций для устранения избыточности (вы можете предпочесть не использовать это как есть, а скорее идею).

Сначала создайте класс, подобный этому:

// Needs argument validation. Also, extending Dictionary<TKey, TValue>
// probably isn't a great idea.
public class ByTypeEvaluator : Dictionary<Type, Func<object, object>>
{
    public void Add<T>(Func<T, object> selector)
    {
        Add(typeof(T), x => selector((T)x));
    }

    public object Evaluate(object key)
    {
        return this[key.GetType()](key);
    }
}

И тогда использование становится:

// Give this variable longer lifetime if you prefer.
var map = new ByTypeEvaluator
{
    (ComboBox c) => c.SelectedItem,
    (TextBox t) => t.Text,
    (DateTimePicker dtp) => dtp.Value,
    (NumericUpDown nud) => nud.Value
};

Control myControl = ...
var myProjection = map.Evaluate(myControl); 
1 голос
/ 21 октября 2011

Я полагаю, вы ищете Шаблон посетителя .Один класс на контроллер является одним из способов сделать это, но процитировать цитируемую статью:

Примечание. Более гибкий подход к этому шаблону состоит в создании класса-оболочки, реализующего интерфейс, определяющий метод accept.Оболочка содержит ссылку, указывающую на CarElement, который может быть инициализирован через конструктор.Этот подход избавляет от необходимости реализовывать интерфейс для каждого элемента.[см. статью Java Tip 98, статья ниже]

Возможно, вам это удастся.

0 голосов
/ 21 октября 2011

вы можете использовать такой подход, опираясь на is:

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

    if (control is TextBox) return (control as TextBox).Text;
    if (control is ComboBox) return (control as ComboBox).SelectedValue;

...
}

и

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

    ... 

    }
}

continue во втором блоке, вероятно, даже не требуется, но это требует некоторого тестирования.

Добро пожаловать на сайт PullRequest, где вы можете задавать вопросы и получать ответы от других членов сообщества.
...