Refactoring an If else tree

.net, c#, refactoring

Solution

what about

singleLoadControl.Visible  = 
      PostingType == PostingTypes.Loads  && !IsMultiPost;      
singleTruckControl.Visible = 
      PostingType == PostingTypes.Trucks && !IsMultiPost;      
multiTruckControl.Visible  = 
      PostingType == PostingTypes.Loads  && IsMultiPost;      
multiLoadControl.Visible   =  
      PostingType == PostingTypes.Trucks && IsMultiPost;

if you want capability to make multiple controls visible (or add more enumerated values) decorate the enum with [Flags] attribute as follows:

[Flags]   
public enum PostTyp { None=0, IsMultiPost = 1, Loads = 2, Trucks = 4 }

and modify Code as follows:

singleLoadControl.Visible  = 
      ((PostingType &  (PostTyp.Loads | ~PostTyp.MultiCast)) 
         == PostingType );      
singleTruckControl.Visible = 
      ((PostingType & (PostTyp.Trucks | ~PostTyp.MultiCast)) 
         == PostingType );          
multiTruckControl.Visible  = 
      ((PostingType & (PostTyp.Loads  |  PostTyp.MultiCast)) 
         == PostingType );        
multiLoadControl.Visible   =  
      ((PostingType & (PostTyp.Trucks |  PostTyp.MultiCast)) 
         == PostingType );

Problem

I have an if else tree that is going to grow as I add additional items for it to maintain and I'm looking at the best way to write it for maintainability I'm starting with this code ``` private void ControlSelect() { if (PostingType == PostingTypes.Loads && !IsMultiPost) { singleLoadControl.Visible = true; singleTruckControl.Visible = false; multiTruckControl.Visible = false; multiLoadControl.Visible = false; } else if (PostingType == PostingTypes.Trucks && !IsMultiPost) { singleLoadControl.Visible = false; singleTruckControl.Visible = true; multiTruckControl.Visible = false; multiLoadControl.Visible = false; } else if (PostingType == PostingTypes.Loads && IsMultiPost) { singleLoadControl.Visible = false; singleTruckControl.Visible = false; multiTruckControl.Visible = false; multiLoadControl.Visible = true; } else if (PostingType == PostingTypes.Trucks && IsMultiPost) { singleLoadControl.Visible = false; singleTruckControl.Visible = false; multiTruckControl.Visible = true; multiLoadControl.Visible = false; } } ``` and thinking of re-factoring it to something like this ``` private void ControlSelect() { List<UserControl> controlList = GetControlList(); string visableControl = singleLoadControl.ID; if (PostingType == PostingTypes.Loads && !IsMultiPost) { visableControl = singleLoadControl.ID; } else if (PostingType == PostingTypes.Trucks && !IsMultiPost) { visableControl = singleTruckControl.ID; } else if (PostingType == PostingTypes.Loads && IsMultiPost) { visableControl = multiLoadControl.ID; } else if (PostingType == PostingTypes.Trucks && IsMultiPost) { visableControl = multiTruckControl.ID; } foreach (UserControl userControl in controlList) { userControl.Visible = (userControl.ID == visableControl); } } private List<UserControl> GetControlList() { List<UserControl> controlList = new List<UserControl> { singleLoadControl, multiTruckControl, singleTruckControl, multiLoadControl }; return controlList; } ``` I take a performance hit but I can manage all of my controls is a single place my other thought was to make each selected control block it own method, something like this ``` private void SetSingleLoadControlAsSelected() { singleLoadControl.Visible = true; singleTruckControl.Visible = false; multiTruckControl.Visible = false; multiLoadControl.Visible = false; } ``` I don't take a performance hit but I'm maintaining the controls in multiple location I'm leaning for option one just because I like maintainability aspect of it.

Original source