【问题标题】:The anti-DRY pattern抗 DRY 模式
【发布时间】:2009-12-22 17:33:22
【问题描述】:

我写了这组代码,感觉很差 在质量上。如您所见,在四个 case 语句中的每一个中 我最终重复了很多相同的代码,除了 在每种情况下都有一些变化。不同的项目;会话名称,网格名称 和 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;
    }
}

【问题讨论】:

  • 什么是ManagerContext.Current.GroupX? (枚举、值等?)
  • 它是 ManagerContext 中的一个属性,它从会话中返回一个 List。 groupX 与 Session["Xteam"] 匹配。如果它不为空,则将其添加到列表中,否则跳过它。
  • 有 Session["deltaTeam"] 好像很奇怪,后来有 Session["deltaGroup"].. 是不是 bug?..
  • 我想我在清理它的时候漏掉了一个名字。我会解决的。
  • 不要硬编码这些字符串,至少应该将它们分配给变量,并使用变量代替它们。防止此类错误;)

标签: c# design-patterns


【解决方案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; 
        } 
} 

这是一种选择,我敢肯定还有进一步的重构。

对于更深入的重构,我会使用选项对象、潜在委托或类似的集合来研究表驱动。它的工作方式是选项将成为一个对象而不是字符串,并且该选项将具有配置它的属性和调用适当委托的方法。这实际上取决于您要维护的抽象级别。有时从常规控件继承并在子类中提供配置信息以便它们知道如何加载自己是值得的。

这里没有足够的空间来真正深入重构。

【讨论】:

  • 我打算输入一种面向对象的方式来执行此操作。这更简单,但如果您有更多案例语句,您可能会考虑创建一个单独的对象来保存这些属性中的每一个。然后为每个单独的选项子类化。
  • @Will,是的,当然,我认为这只是停止重复代码的第一步,然后可能会开始更多的 OO 重构,可能会将配置和控制移动到一个对象中(我将避免使用术语“控制反转”,因为担心人们将基本设计范式与实施 IOC 的过多框架混淆)。我在上次编辑中添加了一条注释。
  • 不错的重构技巧,还是要注意命名。答案中的代码不是 c#。
【解决方案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 可以更改查找组的方式,如果需要更改这些详细信息,我们不必担心查找查找组的所有位置。 GetGroupNameGetGrid 也是如此。更重要的是,如果任何查找逻辑需要在任何地方重用,我们永远不会重复自己。我们非常不受变化影响,在这种情况下永远不会重蹈覆辙。

【讨论】:

  • 仅供参考:我认为您不需要所有那些“新”。
【解决方案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();

}

【讨论】:

  • 我想你的意思是grid.Datasource等
【解决方案4】:

类似的东西应该可以工作:

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

【讨论】:

    【解决方案5】:

    这里只是为了好玩(这意味着它不太可能是一个好主意,并且完全未经测试):

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

    如果我希望能够动态更改(即在运行时)选项匹配集,并且我希望执行的代码(加载算法)是完全开放式的,我只会做这样的事情。

    【讨论】:

    • +1 这个概念是正确的。要扩展它,您只需要在字典中添加一个额外的条目
    【解决方案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();
    }
    

    引发异常的另一种方法是将选项字符串重新输入到枚举中,仅使用您允许的值。这样你就知道如果你得到一个正确的枚举作为输入参数,你的选项就会被处理。

    【讨论】:

      【解决方案7】:

      创建两个函数,GetGroup() 返回类似 ManagerContext.Current.Group4 和 GetGroupName() 的内容,返回类似 "deltaGroup" 的字符串。然后,所有代码都消失了。

      【讨论】:

        【解决方案8】:
            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();
            }   
        
        }
        

        【讨论】:

          猜你喜欢
          • 1970-01-01
          • 2017-12-18
          • 1970-01-01
          • 1970-01-01
          • 1970-01-01
          • 1970-01-01
          • 2012-04-18
          • 2016-02-20
          • 1970-01-01
          相关资源
          最近更新 更多