9
\$\begingroup\$

How can I use a more generic method to clean up the redundancy in the switch statement below?

I'm working on an ASP.NET webforms app that will have five identical user controls that I will be showing and hiding based on the value of a dropdown list. (I have already been down the path of trying to add and remove the controls dynamically--the issue of managing the view states for the controls would have been very complicated, so in the interest of shipping I'm opting for a fixed number of controls.)

The approach I'm currently using works but isn't very elegant: 

protected void DisplayUserControls(int numberOfControls)
{
    switch (numberOfControls)
    {
        case 2:
            UserControl1.Visible = true;
            UserControl2.Visible = true;
            UserControl3.Visible = false;
            break;

        case 3:
            UserControl1.Visible = true;
            UserControl2.Visible = true;
            UserControl3.Visible = true;
            break;

        default:
            UserControl1.Visible = true;
            UserControl2.Visible = false;
            UserControl3.Visible = false;
            break;
    }
}

What are some options here? The best I can come up with is using numberOfControls to build the name of the control, but that seems hacky. Suggestions appreciated.

EDIT: I implemented a solution similar to the accepted answer below. I'm stuck on loading up the controls in the list. The commented out code is more along the lines of what I'd like to do but can't get it working. The uncommented code works. Suggestions? 

private List<ShiftControl> PopulateShiftControlList()
{
    ShiftControl shiftControlList = new List<ShiftControl>();

    //ControlCollection panelControls = ShiftPanel.Controls;

    //foreach (ShiftControl control in panelControls)
    //{
    //    shiftControlList.Add(control);
    //}

    shiftControlList.Add(ShiftControl1);
    shiftControlList.Add(ShiftControl2);
    shiftControlList.Add(ShiftControl3);
    shiftControlList.Add(ShiftControl4);

    return shiftControlList;
}
Improve this question
\$\endgroup\$
2
  • \$\begingroup\$ Please don't use names like UserControl \$\endgroup\$
    – user
    Mar 18, 2012 at 14:49
  • \$\begingroup\$ Yeah... thanks? I don't, and I rarely name programs MyProgram. That was just to make it clear in my post what each item in the case statement was. \$\endgroup\$
    – Josh Earl
    Mar 18, 2012 at 15:08

3 Answers 3

Reset to default
9
\$\begingroup\$

I would create a list of usercontrols. Based on the amount of controls that need to be visible, traverse the list and set x controls to visible.

protected void DisplayUserControls(List<UserControl> controls, int numberOfControls)
{
    Contract.Requires( numberOfControls >= 0 && numberOfControls <= controls.Count );

    for ( int i = 0; i < numberOfControls; ++i )
    {
        controls[ i ].Visible = true;
    }
    for ( int i = numberofControls; i < controls.Count; ++i )
    {
        controls[ i ].Visible = false;
    }
}

Or rather in one go:

protected void DisplayUserControls(List<UserControl> controls, int numberOfControls)
{
    Contract.Requires( numberOfControls >= 0 && numberOfControls <= controls.Count );

    for ( int i = 0; i < controls.Count; ++i )
    {
        controls[ i ].Visible = i < numberOfControls;
    }
}
Improve this answer
\$\endgroup\$
6
  • \$\begingroup\$ I like this a lot... It would also be useful when I need to retrieve the properties of the controls. I was thinking I would have to use a container like a panel and then grab each of the panel's children. The Contract.Requires piece... Can you explain that a little bit? Is it similar to a condensed if...else ? \$\endgroup\$
    – Josh Earl
    Mar 2, 2011 at 15:10
  • \$\begingroup\$ That's a feature of .NET 4.0. It makes sure that when entering the function, the specified constraints are met. It's called Code Contracts. Instead of throwing an out of bounds exception when accessing controls, it will already know something is wrong when verifying the parameters. You could simply replace it with an exception you throw when the arguments aren't valid. \$\endgroup\$
    – Steven Jeuris
    Mar 2, 2011 at 15:13
  • \$\begingroup\$ That's pretty neat. What happens if the constraints aren't met? \$\endgroup\$
    – Michael K
    Mar 2, 2011 at 15:15
  • \$\begingroup\$ @Michael: That depends on how you set up to use code contracts in the environment options. You can either get a runtime exception with the inner condition specified as message, that's already more verbose than an index out of bounds, or you can't compile at all when using what is known as the static checker, and somewhere in your code you make a possible wrong call to this function. Unfortunately that's only available in Visual Studio Ultimate. :) This is an interesting new technology, it's a very interesting read! ;p \$\endgroup\$
    – Steven Jeuris
    Mar 2, 2011 at 15:17
  • \$\begingroup\$ I edited my original post with one more piece that needs work. Any suggestions? \$\endgroup\$
    – Josh Earl
    Mar 2, 2011 at 20:13
3
\$\begingroup\$

Man, all of these answers are way overkill for this. Just do:

protected void DisplayUserControls(int numberOfControls)
{
    UserControl1.Visible = numberOfControls > 0;
    UserControl2.Visible = numberOfControls > 1;
    UserControl3.Visible = numberOfControls > 2;
}

Simple is good.

Improve this answer
\$\endgroup\$
3
  • \$\begingroup\$ Simple is good, but the for loop are also three lines of code, and support as many controls as you want. ... unless you want to argue a for loop is too difficult? ;p \$\endgroup\$
    – Steven Jeuris
    Mar 3, 2011 at 1:38
  • 2
    \$\begingroup\$ This is definitely a simple approach, but it assumes that my code will remain small in scale. However, I simplified the scenario for this post. In my app, there are currently 10 controls across two panels, and there could potentially be more. The other suggestions are more lines of code, but I can add more controls without changing anything and without ending up with a big block of visibility toggles. I do like the way you're using a boolean test as the value for the visibility property; I might end up using that in other contexts. \$\endgroup\$
    – Josh Earl
    Mar 3, 2011 at 13:14
  • \$\begingroup\$ If there are ten controls, make a collection and do a loop. If there were three I'd just do the above. Different scales call for different solutions. I wouldn't worry about "potentially be more". If there aren't more now just do the simple solution. That simpler code will be easy to change later if you need more and if you don't then you've kept things simple. \$\endgroup\$
    – munificent
    Mar 3, 2011 at 19:46
1
\$\begingroup\$

First thought:

UserControl1.Visible = true;
UserControl2.Visible = true;
UserControl3.Visible = true;

switch (numberOfControls)
{
   case 1:
      UserControl2.Visible = false;
      UserControl3.Visible = false;
      break;
   case 2:
      UserControl3.Visible = false;
      break;
   case 3:
}

EDIT: New idea:

Arraylist controls;

for (int x=0; x<numberOfControls; x++)
{
   ((Control)controls.get(x)).Visible = true;
}
Improve this answer
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

Not the answer you're looking for? Browse other questions tagged or ask your own question.