Анти-сухой шаблон

Question

Я написал этот набор кода и чувствую, что он довольно плохой в качестве. Как видите, в каждом из четырех случаев заявления Я заканчиваю тем, что повторяю очень много одного и того же кода, за исключением несколько вариантов в каждом случае. Предметы, которые варьируются; имена сеансов, имена сеток и имя группы ManagerContext. Кто-нибудь может взять этот беспорядок кода и покажи мне лучший способ сделать это?

private void LoadGroup(string option)
{
    switch (option.ToUpper())
    {
        case "ALPHA":
            VList<T> alphaList = FetchInformation(
                                   ManagerContext.Current.Group1);

            if (Session["alphaGroup"] != null)
            {
                List<T> tempList = (List<T>)Session["alphaGroup"];
                alphaList.AddRange(tempList);
            }
            uxAlphaGrid.DataSource = alphaList;
            uxAlphaGrid.DataBind();
            break;
        case "BRAVO":
            VList<T> bravoList = FetchInformation(
                                   ManagerContext.Current.Group2);

            if (Session["bravoGroup"] != null)
            {
                List<T> tempList = (List<T>)Session["bravoGroup"];
                bravoList.AddRange(tempList);
            }
            uxBravoGrid.DataSource = bravoList;
            uxBravoGrid.DataBind();
            break;
        case "CHARLIE":
            VList<T> charlieList = FetchInformation(
                                   ManagerContext.Current.Group3);

            if (Session["charlieGroup"] != null)
            {
                List<T> tempList = (List<T>)Session["charlieGroup"];
                charlieList.AddRange(tempList);
            }
            uxCharlieGrid.DataSource = charlieList;
            uxCharlieGrid.DataBind();
            break;
        case "DELTA":
            VList<T> deltaList = FetchInformation(
                                   ManagerContext.Current.Group4);

            if (Session["deltaGroup"] != null)
            {
                List<T> tempList = (List<T>)Session["deltaGroup"];
                deltaList.AddRange(tempList);
            }
            uxDeltaGrid.DataSource = deltaList;
            uxDeltaGrid.DataBind();
            break;
        default:                
            break;
    }
}

Answer 1

Вы должны быть в состоянии извлечь части корпуса в параметризованную вспомогательную функцию:

function helper(grp, grpname, dg) {
    VList<T> theList = FetchInformation(grp); 
    if (Session[grpname] != null) 
    { 
        List<T> tempList = (List<T>)Session[grpname]; 
        theList.AddRange(tempList); 
    } 
    dg.DataSource = theList; 
    dg.DataBind(); 
}

private void LoadGroup(string option) 
{ 
        switch (option.ToUpper()) 
        { 
                case "ALPHA": 
                        helper(ManagerContext.Current.Group1, "alphaGroup", uxAlphaGrid);
                        break; 
                case "BRAVO": 
                        helper(ManagerContext.Current.Group2, "bravoGroup", uxBravoGrid);
                        break; 
                case "CHARLIE": 
                        helper(ManagerContext.Current.Group3, "charlieGroup", uxCharlieGrid);
                        break; 
                case "DELTA": 
                        helper(ManagerContext.Current.Group4, "deltaGroup", uxDeltaGrid);
                        break; 
                default:                                 
                        break; 
        } 
} 

Это один из вариантов, и я уверен, что есть дальнейший рефакторинг.

Для более глубокого рефакторинга я бы посмотрел на управление таблицами с использованием набора опционных объектов, потенциально делегатов или подобных объектов. Это работает так, что параметр становится объектом, а не строкой, а параметр имеет свойства, которые его настраивают, и методы, которые вызывают соответствующие делегаты. это действительно зависит от уровня абстракции, который вы хотите поддерживать. Иногда стоит унаследовать от обычных элементов управления и предоставить информацию о конфигурации в подклассе, чтобы они знали, как загрузить себя.

Здесь недостаточно места, чтобы действительно углубиться в эту глубину рефакторинга.

Answer 2

Я бы предпочел что-то вроде этого:

private void LoadGroup(string option) {
    Group group = GetGroup(option);
    string groupName = GetGroupName(option);
    Grid grid = GetGrid(option);

    BindGroup(group, groupName, grid);
}

Group GetGroup(string option) {
    // ideally this should be defined and initialized elsewhere
    var dictionary = new Dictionary<string, Group>() {
        { "ALPHA", ManagerContext.Current.Group1 },
        { "BETA", ManagerContext.Current.Group2 },
        { "CHARLIE", ManagerContext.Current.Group3 },
        { "DELTA", ManagerContext.Current.Group4 }
    };   

    return dictionary[option.ToUpperInvariant()];
}

string GetGroupName(string option) {
    return option.ToLowerInvariant() + "Group";
}

Grid GetGrid(string option) {
    // ideally this should be defined and initialized elsewhere
    var dictionary = new Dictionary<string, Grid>() {
        { "ALPHA", uxAlphaGrid },
        { "BETA", uxBetaGrid },
        { "CHARLIE", uxCharlieGrid },
        { "DELTA", uxDeltaGrid }
    };

    return dictionary[option.ToUpperInvariant()];
}

void BindGroup(Group group, string groupName, Grid grid) {
    VList<T> groupList = FetchInformation(group);
    if (Session[groupName] != null) {
        List<T> tempList = (List<T>)Session[groupName];
        groupList.AddRange(tempList);
    }
    grid.DataSource = groupList;
    grid.DataBind();
}

А теперь посмотрите, как хорошо мы изолированы от изменений. GetGroup, например, может изменить способ поиска групп, и нам не нужно беспокоиться о поиске всех мест, где ищутся группы, в случае необходимости изменения этих деталей. Аналогично для GetGroupName и GetGrid. Более того, мы никогда не повторяем себя, если какая-либо логика поиска должна быть повторно использована где-либо. Мы очень изолированы от перемен и никогда не повторим себя, если учтем это так.

Answer 3

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

И так:

private void LoadGroup(string option)
{
        switch (option.ToUpper())
        {
                case "ALPHA":
                        BindData("alphaGroup", uxAlphaGrid, FetchInformation(ManagerContext.Current.Group1));
                        break;
                case "BRAVO":
                        BindData("bravoGroup", uxBravoGrid, FetchInformation(ManagerContext.Current.Group2));
                        break;
                case "CHARLIE":
                        BindData("charlieGroup", uxCharlieGrid, FetchInformation(ManagerContext.Current.Group3));
                        break;
                case "DELTA":
                        BindData("deltaTeam", uxDeltaGrid, FetchInformation(ManagerContext.Current.Group4));                        
                        break;
                default:                                
                        break;
        }
}

private void BindData(string sessionName, GridView grid, VList<T> data) // I'm assuming GridView here; dunno the type, but it looks like they're shared
{
    if (Session[sessionName] != null)
    {
            List<T> tempList = (List<T>)Session[sessionName];
            data.AddRange(tempList);
    }
    grid.DataSource = data;
    grid.DataBind();

}

Answer 4

Вот только для развлечения (то есть вряд ли это будет хорошая идея, и она полностью не проверена):

public class YourClass
{
    private Dictionary<string, Action> m_options;

    public YourClass()
    {
     m_options = new Dictionary<string, Action>
     {
      { "ALPHA",  () => LoadGroup(ManagerContext.Current.Group1, "alphaGroup", uxAlphaGrid) },
      { "BRAVO",  () => LoadGroup(ManagerContext.Current.Group2, "bravoGroup", uxBravoGrid) },
      { "CHARLIE",() => LoadGroup(ManagerContext.Current.Group3, "charlieGroup", uxCharlieGrid) },
      { "DELTA",  () => LoadGroup(ManagerContext.Current.Group4, "deltaGroup", uxDeltaGrid) },
     };
    }

    private void LoadGroup(string option)
    {
     Action optionAction;

     if(m_options.TryGetValue(option, out optionAction))
     {
            optionAction();
     }
    }

    private void LoadGroup(TGroup group, string groupName, TGrid grid)
    {
        VList<T> returnList = FetchInformation(group);

        if (Session[groupName] != null)
        {
                List<T> tempList = (List<T>)Session[groupName];
                returnList.AddRange(tempList);
        }
        grid.DataSource = returnList;
        grid.DataBind();
    }
}

Я бы сделал что-то подобное, только если бы я хотел иметь возможность динамически изменять (т. Е. Во время выполнения) набор параметров, и хотел, чтобы исполняемый код (алгоритм загрузки) был полностью открытым.

Answer 5

Что-то похожее на это должно работать:

private void LoadGroup(string option)
{
        switch (option.ToUpper())
        {
                case "ALPHA":
                        BindGroup(ManagerContext.Current.Group1, "alphaGroup", uxAlphaGrid);
                        break;
                case "BRAVO":
                        BindGroup(ManagerContext.Current.Group2, "bravoGroup", uxBravoGrid);
                        break;
                case "CHARLIE":
                        BindGroup(ManagerContext.Current.Group3, "charlieGroup", uxCharlieGrid);
                        break;
                case "DELTA":
                        BindGroup(ManagerContext.Current.Group4, "deltaGroup", uxDeltaGrid);
                        break;
                default:                                
                        break;
        }
}

private void BindGroup(GroupType group, string groupName, GridView grid)
{
    VList<T> vList = FetchInformation(group);

    if (Session[groupName] != null)
    {
        List<T> tempList = (List<T>)Session[groupName];
        vList.AddRange(tempList);
    }
    grid.DataSource = vList;
    grid.DataBind();
}

Answer 6

private void LoadGroup(string option)
{
    option = option.ToLower();
    sessionContent = Session[option + "Group"];

    switch(option)
    {
        case "alpha":
            var grp = ManagerContext.Current.Group1;
            var grid = uxAlphaGrid;
            break;
        case "bravo":
            var grp = ManagerContext.Current.Group2;
            var grid = uxBravoGrid;
            break;
        case "charlie":
            var grp = ManagerContext.Current.Group3;
            var grid = uxCharlieGrid;
            break;
        // Add more cases if necessary
        default:
            throw new ArgumentException("option", "Non-allowed value");
    }

    VList<T> groupList = FetchInformation(grp);
    if (sessionContent != null)
    {
        List<T> tempList = (List<T>)sessionContent;
        groupList.AddRange(tempList);
    }

    grid.DataSource = List("alpha";
    grid.DataBind();
}

Альтернативой выбрасыванию исключения является повторный ввод строки параметра в Enum только с допустимыми значениями. Таким образом, вы знаете, что если вы получите правильное перечисление в качестве входного аргумента, ваш параметр будет обработан.

Answer 7

    private void LoadGroup(GridView gv, string groupName, ManagerContext m)
{
    VList<T> list = FetchInformation(m); //not sure if ManagerContext will get what you need
    if (Session[groupName] != null)
    {
       list.AddRange(List<T>Session[groupName]);
       gv.DataSource = list;
       gv.DataBind();
    }   

}

Answer 8

Создайте две функции, GetGroup(), возвращающие такие вещи, как ManagerContext.Current.Group4 и GetGroupName (), возвращающие строки, такие как "deltaGroup". Затем весь код исчезнет.