Рефакторинг моего кода на C #: оператор if-else и повторение кода

Question

Дано:

private void UpdataFieldItems(List<FieldItemBase> existingFieldItems, List<FieldItemBase> selectedFieldItems)
    {
        List<FieldItemBase> newFieldItemsSelected;
        var fieldSelectionChanges = GetFieldSelectionChanges(out newFieldItemsSelected);//retuns a Flagged enum

        if (Coding.EnumHas(fieldSelectionChanges, FieldSelectionChanges.AddedFieldItem))
        {
            StartEditMode();
            SetColumnDescriptorsToAdd(newFieldItemsSelected);
            UpdateItems(selectedFieldItems);

            SetColumnsToShow();
            CustomizeAlignmentAndCellFormatters(_tfaTableGrid.TableGrid);

            if (_tfaTableGrid.TableGrid.ColumnDescriptors.Count() > 0)
            {
                SetAdditionalFirstGroupedColumn();
            }

            StopEditMode();
        }

        else if (Coding.EnumHas(fieldSelectionChanges, FieldSelectionChanges.Summary))
        {
            StartEditMode();

            UpdateItems(fieldItems);
            SetColumnsToShow();

            if (_tfaTableGrid.TableGrid.ColumnDescriptors.Count() > 0)
            {
                SetAdditionalFirstGroupedColumn();
            }

            StopEditMode();

        }

        else if (Coding.EnumHas(fieldSelectionChanges, FieldSelectionChanges.Order) ||
                 Coding.EnumHas(fieldSelectionChanges, FieldSelectionChanges.AddedCustomFieldItem) ||
                 Coding.EnumHas(fieldSelectionChanges, FieldSelectionChanges.RemovedItem))
        {
            UpdateItems(fieldItems);
            SetColumnsToShow();

        }
            Invalidate();
    }

//Coding.cs
public static bool EnumHas(FieldSelectionChanges selectionChanges, FieldSelectionChanges valueToCheck)
        {
            return (selectionChanges & valueToCheck) == valueToCheck;
        }

Я готов провести рефакторинг вышеуказанного кода.В вышеприведенном коде есть две вещи, которые мне не нравятся:

1) Запись одних и тех же вызовов методов в разных случаях, из этих случаев невозможно извлечь вызовы общих методов.

2) Читаемость этого кода очень плохая.Было бы очень сложно понять и отладить, если позже понадобится.

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

Спасибо за ваш интерес.

Answer 1

1. Используйте метод извлечения для тела каждого оператора if
2. Создать словарь>, чтобы выбрать соответствующее действие для fieldSelectionChanges.Это шаблон стратегии

Answer 2

Я бы сделал различные условия более выразительными с точки зрения того, что они на самом деле делают с вашим приложением, если вы уже используете достаточно описательные имена методов для действий. Что-то вроде:

        private void UpdataFieldItems(List<FieldItemBase> existingFieldItems, List<FieldItemBase> selectedFieldItems)
        {
            List<FieldItemBase> newFieldItemsSelected;
            var fieldSelectionChanges = GetFieldSelectionChanges(out newFieldItemsSelected);//retuns a Flagged enum

            if (IsValidChange(fieldSelectionChanges))
            {
                List<FieldItemBase> targetfields = null;
                if (IsInEditMode(fieldSelectionChanges))
                    StartEditMode();

                if (IsItemAdded(fieldSelectionChanges))
                {
                    SetColumnDescriptorsToAdd(newFieldItemsSelected);
                    targetFields = selectedFieldItems;
                }
                else
                    targetFields = existingFieldItems;

                UpdateItems(targetFields);
                SetColumnsToShow();

                if (IsItemAdded(fieldSelectionChanges))
                    CustomizeAlignmentAndCellFormatters(_tfaTableGrid.TableGrid);

                if (IsInEditMode(fieldSelectionChanges))
                {
                    if (_tfaTableGrid.TableGrid.ColumnDescriptors.Count() > 0)
                        SetAdditionalFirstGroupedColumn();
                    StopEditMode();
                }
            }

            Invalidate();
        }

        private bool InEditMode(FlaggedEnum fieldSelectionChanges)
        {
            return Coding.EnumHas(fieldSelectionChanges, FieldSelectionChanges.Summary) || Coding.EnumHas(fieldSelectionChanges, FieldSelectionChanges.AddedFieldItem);
        }

        private bool IsItemAdded(FlaggedEnum fieldSelectionChanges)
        {
            Coding.EnumHas(Coding.EnumHas(fieldSelectionChanges, FieldSelectionChanges.AddedFieldItem);
        }

        private bool IsValidChange(FlaggedEnum fieldSelectionChanges)
        {
            return Coding.EnumHas(fieldSelectionChanges, FieldSelectionChanges.Order) ||
                   Coding.EnumHas(fieldSelectionChanges, FieldSelectionChanges.AddedCustomFieldItem) ||
                   Coding.EnumHas(fieldSelectionChanges, FieldSelectionChanges.RemovedItem) ||
                   Coding.EnumHas(fieldSelectionChanges, FieldSelectionChanges.Summary) ||
                   Coding.EnumHas(fieldSelectionChanges, FieldSelectionChanges.AddedFieldItem);
        }

        //Coding.cs
        public static bool EnumHas(FieldSelectionChanges selectionChanges, FieldSelectionChanges valueToCheck)
        {
            return (selectionChanges & valueToCheck) == valueToCheck;
        }

Answer 3

Хорошо, что повторяющаяся / несколько уродливая часть - это операторы IF.

Предложите сохранить результат этих условий IF в логической переменной и использовать это.

Этот код не завершен, но вы поняли.

        bool scenarioOne = Coding.EnumHas(fieldSelectionChanges, FieldSelectionChanges.AddedFieldItem);
        bool scenarioTwo = Coding.EnumHas(fieldSelectionChanges, FieldSelectionChanges.Summary);
        bool scenarioThree = Coding.EnumHas(fieldSelectionChanges, FieldSelectionChanges.Order) || Coding.EnumHas(fieldSelectionChanges, FieldSelectionChanges.AddedCustomFieldItem) || Coding.EnumHas(fieldSelectionChanges,FieldSelectionChanges.RemovedItem);

        if (scenarioOne || scenarioTwo)
            StartEditMode();

        if (scenarioOne) {
            SetColumnDescriptorsToAdd(newFieldItemsSelected);
            UpdateItems(selectedFieldItems);
        }
        else if (scenarioTwo || scenarioThree) {
            UpdateItems(fieldItems);
        }

        if (scenarioOne || scenarioTwo || scenarioThree)
            SetColumnsToShow();

Очевидно, выбирайте более подходящие имена для переменной.

Или, еще лучше, разделитесь на отдельные методы.

Answer 4

Я бы предложил выделить три блока в отдельные, хорошо названные методы.Это улучшит читабельность, если метод UpdateFieldItems.Поскольку эти три блока совершенно разные, у них нет никакой возможности объединить их дальше.