Tuesday, March 1, 2011

Refactoring solution needed

Hi, I have a page on which I must load controls dynamically based on the user selection. Let's say that I have something like this:

public static readonly Dictionary<string, string> DynamicControls = new Dictionary<string, string>
        {
            { "UserCtrl1",  "~/Controls/UserCtrl1.ascx" },
            { "UserCtrl2",  "~/Controls/UserCtrl2.ascx" },
            { "UserCtrl3",  "~/Controls/UserCtrl3.ascx" },
            { "UserCtrl4",  "~/Controls/UserCtrl4.ascx"}
};

Now let's say than on the page where the controls are loaded the code is something like this:

protected void Page_Load(object sender, EventArgs e)
        {
            SomePanel.Controls.Add(GetControl());
        }        

        private Control GetControl()
        {
            string dynamicCtrl = CurrentItem.DynamicControl;
            string path = SomeClass.DynamicControls[dynamicCtrl];

            Control ctrl = null;            

            //TODO: find a better way to load the controls
            switch (dynamicCtrl)
            {
                case "UserCtrl1":
                    {
                        ctrl = (UserCtrl1)LoadControl(path);
                    }
                    break;
                case "UserCtrl2":
                    {
                        ctrl = (UserCtrl2)LoadControl(path);
                    }
                    break;
                case "UserCtrl3":
                    {
                        ctrl = (UserCtrl3)LoadControl(path);
                    }
                    break;
                default:
                    {
                        throw new ApplicationException("Invalid dynamic control added.");
                    }                
            }

            return ctrl;
        }

The page has the required registered statements. Any idea how I can get rid of this ugly switch statement?

From stackoverflow
  • Can you not just use foreach on your dictionary and do your test and LoadControl in there?

  • That wont help because the switch is needed for casting to the right control type.

  • You don't need to cast the result from LoadControl.

    This should do:

    private Control GetControl()
    {
        string dynamicCtrl = CurrentItem.DynamicControl;
        string path = SomeClass.DynamicControls[dynamicCtrl];
    
        Control ctrl = LoadControl(path);    
    
        return ctrl;
    }
    
    Gishu : You probably also want to run the 'Inline Temp with Query' refactoring in there.. I'd say nix the dynamicCtrl and ctrl variables.. path aids readability so I would keep it. e.g. return LoadControl(path)
  • You probably want something like this (pseudo-ish code):

    foreach key in dictionary
       if key = dynamicControl then
          ctrl = (Type.GetType(key))LoadControl(dictionary.get(key))
       end if
    next
    

0 comments:

Post a Comment