CODEWITHSUNDEEP
c#switch-statement
Impostor
Impostor

Reputation: 2050

Refactoring switch statement

I have this switch statement

string status = "1";
switch (status)
{
    case "1":
        Button1.Visible = true;
        Button2.Visible = true;
        Button3.Visible = true;
        Button4.Visible = true;
        Button5.Visible = true;
        Panel1.Visible = true;
        break;
    case "2":
        Button1.Visible = true;
        Button2.Visible = true;
        Button3.Visible = true;
        Button4.Visible = true;
        Button5.Visible = true;
        Panel2.Visible = true;
        break;
}

and there is some code redundancy. In both cases Button1 - 5 are shown and the Panel referring to the status value.

Is there a way to make the code shorter?

My approach with less redundancy but more lines - is there another obvious way I haven't thought of?

string status = "1";
switch (status)
{
    case "1":
    case "2":
        Button1.Visible = true;
        Button2.Visible = true;
        Button3.Visible = true;
        Button4.Visible = true;
        Button5.Visible = true;
        break;
}
switch (status)
{
    case "1":
        Panel1.Visible = true;
        break;
    case "2":
        Panel2.Visible = true;
        break;
}

Upvotes: 1

Views: 153

Answers (5)

Damien_The_Unbeliever
Damien_The_Unbeliever

Reputation: 239814

One interesting approach you might consider taking is to go declarative.

Let's create a class, we'll call it State:

public class State {
    public string Status {get; set;}
}

The idea being, that you put anything in this class that make affect UI state. At the moment, we just have the Status property.

Then, you create some "behaviour" methods (these could just be lambdas; All we want/need is that they're generally reusable and can satisfy Action<State,Control>):

private void VisibleOn1Or2(State state, Control control)
{
   control.Visible = (state.Status == "1" || state.Status == "2");
}
private void VisibleOn1(State state, Control control)
{
   control.Visible = (state.Status == "1");
}
//Etc

Then, create a Dictionary<Control,List<Action<State,Control>>> collection (I'll call it uiUpdates below), and add all of the behaviours that you want. Note that this design allows you to have multiple cracks at each control - maybe you want to affect Font based on State as well as Visible and you can use two small methods to do this if desired rather than a single method that deals with both.

This Dictionary should be populated straight after InitializeControls has finished working.

Note now you can have a single method:

private void AfterStateUpdated(State newState)
{
   foreach(var kvp in uiUpdates)
   {
      var c = kvp.Key;
      foreach(var update in kvp.Value)
      {
         update(newState,c);
      }
   }
}

And you call that whenever your state changes. Now you don't have to have a gloryhole method than knows exactly how to update all UI state based on all possible State values.

(In the above, I assume there's a single State instance in a form field as well, but I still prefer to pass the State explicitly to the Action methods rather than having them pick it up from the Form. This enhances opportunities for reuse if you want to have a central set of methods used by multiple forms (assuming that the State definition is also suitable for such sharing))

Upvotes: 0

Di Kamal
Di Kamal

Reputation: 173

You can create new method to handle visibility status for the buttons as below:

    private void ChangeButtonVisibility(bool Status)
    {
        Button1.Visible = Status;
        Button2.Visible = Status;
        Button3.Visible = Status;
        Button4.Visible = Status;
        Button5.Visible = Status;
        Panel1.Visible = Status;
    }

Then from switch statement just call the method with proper value as below:

        switch (status)
        {
            case "1":
                ChangeButtonVisibility(true);
                break;
            case "2":
                ChangeButtonVisibility(false);
                break;
        }

Upvotes: -1

Antoine V
Antoine V

Reputation: 7204

Put the buttons to a list and use ForEach to set property

var listButton = new List<Button> {button1, button2, ...}

switch (status)
{
    case "1":
       listButton.ForEach(b => b.Visible = true);
       Panel1.Visible = true;
       break;
    case "2":
       listButton.ForEach(b => b.Visible = true);
       Panel2.Visible = true;
       break;
}

Upvotes: 0

kademat
kademat

Reputation: 134

What about making a function that switch all needed things? If you add something then you will only edit that function keeping your switch clean:

string status = "1";
switch (status)
{
    case "1":
        enableSomething(true);
        break;
    case "2":
        enableSomething(false);
        break;
}

And then:

enableSomething(bool switcher) {
    switcher ? Panel2.Visible = true : Panel1.Visible = true;
    Button1.Visible = true;
    Button2.Visible = true;
    Button3.Visible = true;
    Button4.Visible = true;
    Button5.Visible = true;
}

Upvotes: 0

JuanR
JuanR

Reputation: 7803

Create a method for button visibility:

private void SetButtonVisibility(bool show)
{
    Button1.Visible = show;
    Button2.Visible = show;
    Button3.Visible = show;
    Button4.Visible = show;
    Button5.Visible = show;
}

Then call this method in your switch statement:

switch (status)
{
    case "1":
        SetButtonVisibility(true);
        Panel1.Visible = true;
        break;
    case "2":
        SetButtonVisibility(true);
        Panel2.Visible = true;
        break;
}

Upvotes: 6

Related Questions