Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

The anti-DRY pattern

I've written this set of code and feel that it's pretty poor in quality. As you can see, in each of the four case statements I'm ending up repeating an awful lot of the same code except for a few variations in each case. Items that vary; session names, gridnames and the ManagerContext group name. Can anyone take this mess of code and show me a better way of doing this?

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;
    }
}
like image 410
Chris Avatar asked Nov 27 '22 21:11

Chris


1 Answers

You should be able to extract the parts of the case to a parameterized helper function:

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

That's one option and there is further refactoring, I'm sure.

For deeper refactorings, I would look at table-driving using a collection of option objects, potentially delegates, or similar. The way this works is that the option would become an object instead of a string and the option would have properties which configure it and methods which invoke the proper delegates. it really depends on the abstraction level you want to maintain. Sometimes it pays to inherit from the regular controls and provide configuration information in the subclass so that they know how to load themselves.

There is not enough space here to really go into that depth of refactoring.

like image 57
Cade Roux Avatar answered Dec 16 '22 20:12

Cade Roux