Рефакторинг c#: как удалить повторяющийся код, когда свойства для обновления различаются в каждом случае

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

например - представьте, что у нас есть куча цветных полей для обновления - у меня могут быть некоторые методы, которые выглядят так:

        protected void SetBlueBoxVisibility(bool blueBoxVisibility)
    {
        Project project = LoadProject();
        project.ShowBlueBox = blueBoxVisibility
        ReDrawSomeThings();
        CalcualteSomeStuff();
        Project.UpdateBoxStatus();
        SaveProject(project);
        ShowBlueBoxPanel(blueBoxVisibility);
        RaiseStatusUpdated();
    }

    protected void SetRedBoxVisibility(bool redBoxVisibility)
    {
        Project project = LoadProject();
        project.ShowRedBox = redBoxVisibility
        ReDrawSomeThings();
        CalcualteSomeStuff();
        Project.UpdateBoxStatus();
        SaveProject(project);
        ShowRedBoxPanel(redBoxVisibility);
        RaiseStatusUpdated();

    }

Теперь, очевидно - большая часть этого материала повторяется - что является болью, когда я прихожу, чтобы что-то изменить. Особенно, если у меня будет двадцать разных цветов коробок, а не два!

Я думал, что должен быть способ удалить код, который изменяется, и собрать то же самое в более общем методе, но у меня возникли проблемы с тем, как это сделать.

Я слышал о закрытии, но я недостаточно разобрался с ними, чтобы знать, помогут ли они здесь.

Я подумал, что возможно следующее может быть в правильной строке, но я не знаю, как указать универсальному методу, с каким свойством работать - [Project Variable To Update]

        protected void SetRedBoxVisibility(bool redBoxVisibility)
    {
        SetGenericBoxVisibility([Project Variable To Update],redBoxVisibility)
        ShowRedBoxPanel(redBoxVisibility);
        RaiseStatusUpdated();   
    }

    protected void SetBlueBoxVisibility(bool blueBoxVisibility)
    {
        SetGenericBoxVisibility([Project Variable To Update],blueBoxVisibility)
        ShowBlueBoxPanel(blueBoxVisibility);
        RaiseStatusUpdated();   
    }

    protected void SetGenericBoxVisibility([Project Variable To Update], boxVisibility)
    {
        Project project = LoadProject();
        project.**[Project Variable To Update]** = boxVisibility
        ReDrawSomeThings();
        CalcualteSomeStuff();
        Project.UpdateBoxStatus();
        SaveProject(project);
    }

Любые указатели на то, как обращаться с такими вещами, были бы полезны :)


c# code-duplication refactoring
person Pete McPhearson    schedule 07.09.2011    source источник
comment
Вы должны прочитать об основных принципах ООП.   -  person Dave Ziegler    schedule 07.09.2011
comment
@ Джек - может быть, вам следует дать какое-то руководство или хотя бы указать, в чем, по вашему мнению, может заключаться проблема.   -  person Kieren Johnstone    schedule 07.09.2011

Ответы (3)


arrow_upward
3
arrow_downward

Ну, вы можете извлечь его следующим образом:

protected void SetGenericBoxVisibility(Action<Project> propertySetter,
                                       Action<bool> panelShower,
                                       bool boxVisibility)
{
    Project project = LoadProject();
    propertySetter(project);
    ReDrawSomeThings();
    CalculateSomeStuff();
    Project.UpdateBoxStatus();
    SaveProject(project);
    panelShower();
    RaiseStatusUpdated();
}

Затем:

protected void SetBlueBoxVisibility(bool blueBoxVisibility)
{
    SetGenericBoxVisibility(project => project.ShowBlueBox = blueBoxVisibility,
                            () => ShowBlueBoxPanel(blueBoxVisibility));
}

protected void SetRedBoxVisibility(bool redBoxVisibility)
{
    SetGenericBoxVisibility(project => project.ShowRedBox = redBoxVisibility,
                            () => ShowRedBoxPanel(redBoxVisibility));
}

Правда, это не ужасно приятно...

person Jon Skeet    schedule 07.09.2011
comment
Спасибо :-) Я немного поигрался с некоторым кодом, и это выглядит действительно полезным - это новый синтаксис для меня, чтобы исследовать - теперь я пойду и использую то, что я узнал, для чего-то более захватывающего, чем показ цветные коробочки :-) - person Pete McPhearson; 08.09.2011
comment
@Girl Called Pete: О да, лямбда-выражения невероятно полезны :) - person Jon Skeet; 08.09.2011

arrow_upward
1
arrow_downward

Я думаю, у вас могут быть большие проблемы - один метод обновления на коробку просто не подходит. У вас есть SetGenericBoxVisibility, но затем вы отменяете любую хорошую работу, продолжая использовать методы Set*BoxVisibility. Я не знаю, какую технологию вы используете - если это WPF, посмотрите на MVVM, тогда вы можете просто обновить свои ViewModels. Если это WinForms, возможно, вам следует создать какой-то словарь - Dictionary<BoxType, Box> _boxLookup, где BoxType - это перечисление определяемых вами типов. Затем, чтобы установить видимость блока, вы делаете _boxLookup[BoxType.Red].Property = value; или у вас могут быть методы для управления блоком, принимающие параметр BoxType.

Однако дополнительный контекст был бы очень полезен, так как даже это решение не идеально. Перемещение некоторого другого кода или получение некоторого представления о более широкой картине проблемы, которую вы решаете с помощью нескольких блоков, должно привести к еще нескольким моментам «ага».

person Kieren Johnstone    schedule 07.09.2011
comment
:) На самом деле я просто изучаю принципы (кажется, есть много способов содрать шкуру с кошки). Фактический контекст, в котором это произошло, заключался в наличии набора флажков в веб-форме и использовании метода OnCheckedChanged в каждом поле для выполнения некоторой обработки (включая обновление логического флага на объекте). Наличие одного общего метода обновления флажка для обработки события может быть выходом — я думаю, следующий трюк для этого — определить, какой флаг обновлять, глядя на отправителя, который передается событию. - person Pete McPhearson; 08.09.2011

arrow_upward
1
arrow_downward

Я думаю, что для реального рефакторинга вам нужно сделать интерфейс IBox. По сути, этот интерфейс действует как контракт, определяющий, какие методы и свойства должны быть у всех объектов Box, по крайней мере.

interface IBox {
 //your generic properties and method stubs
 Bool visibility;
}

Теперь реализуйте интерфейс для каждого из ваших «ящиков».

class blueBox : IBox
{
 //here you will have your concrete implementations of the above methods and properties
public Bool visibility   {get; set;} // this doesn't make sense with auto getter setters. you would need to write your bluebox specific getter and setters

}

class redBox : IBox
{
//more concrete implementation

}


public myMethod_To_Do_Stuff(IBox myBox) { // see how I am passing the interface not the conrete classes

myBox.visibility = true;

}
person Doug Chamberlain    schedule 07.09.2011