Switch statement to build context menu (with many redundant lines of code)

Question

I have switch statement that is switching on an enum to build a ContextMenu depending on what the selected enum value is. While the various enum values require different operations in their cases of the switch, some of them require some of the same operations.

All of the MenuItems are declared as readonly and initialized in the constructor of the class. This method is called whenever the QueryType value changes, and is set to a property that is bound to by a ContextMenu in my view.

Switching to if/else would allow for 'circuit simplification' per se, but at the expense of some readability (and the ease of ordering the MenuItems).

private List<MenuItem> GenerateContextMenuItems()
{
 var contextMenuItems = new List<MenuItem>();
 switch (QueryType)
 {
 case QueryType.ContextComputerGroups:
 contextMenuItems.Add(_groupGetComputersMenuItem);
 contextMenuItems.Add(_groupGetUsersMenuItem);
 contextMenuItems.Add(_groupGetUsersDirectReportsMenuItem);
 contextMenuItems.Add(_groupGetUsersGroupsMenuItem);
 break;
 case QueryType.ContextDirectReportDirectReports:
 contextMenuItems.Add(
 _directReportGetDirectReportsMenuItem);
 contextMenuItems.Add(_directReportGetGroupsMenuItem);
 contextMenuItems.Add(_directReportGetSummaryMenuItem);
 contextMenuItems.Add(_userGetGroupsMenuItem);
 contextMenuItems.Add(_userGetSummaryMenuItem);
 break;
 case QueryType.ContextDirectReportGroups:
 contextMenuItems.Add(_groupGetComputersMenuItem);
 contextMenuItems.Add(_groupGetUsersMenuItem);
 contextMenuItems.Add(_groupGetUsersDirectReportsMenuItem);
 contextMenuItems.Add(_groupGetUsersGroupsMenuItem);
 contextMenuItems.Add(_userGetDirectReportsMenuItem);
 contextMenuItems.Add(_userGetSummaryMenuItem);
 break;
 case QueryType.ContextGroupComputers:
 contextMenuItems.Add(_computerGetGroupsMenuItem);
 contextMenuItems.Add(_groupGetUsersMenuItem);
 contextMenuItems.Add(_groupGetUsersDirectReportsMenuItem);
 contextMenuItems.Add(_groupGetUsersGroupsMenuItem);
 break;
 case QueryType.ContextGroupUsers:
 contextMenuItems.Add(_groupGetComputersMenuItem);
 contextMenuItems.Add(_groupGetUsersDirectReportsMenuItem);
 contextMenuItems.Add(_groupGetUsersGroupsMenuItem);
 contextMenuItems.Add(_userGetDirectReportsMenuItem);
 contextMenuItems.Add(_userGetGroupsMenuItem);
 contextMenuItems.Add(_userGetSummaryMenuItem);
 break;
 case QueryType.ContextGroupUsersDirectReports:
 contextMenuItems.Add(
 _directReportGetDirectReportsMenuItem);
 contextMenuItems.Add(_directReportGetGroupsMenuItem);
 contextMenuItems.Add(_directReportGetSummaryMenuItem);
 contextMenuItems.Add(_groupGetComputersMenuItem);
 contextMenuItems.Add(_groupGetUsersMenuItem);
 contextMenuItems.Add(_groupGetUsersGroupsMenuItem);
 contextMenuItems.Add(_userGetDirectReportsMenuItem);
 contextMenuItems.Add(_userGetGroupsMenuItem);
 contextMenuItems.Add(_userGetSummaryMenuItem);
 break;
 case QueryType.ContextGroupUsersGroups:
 contextMenuItems.Add(_groupGetComputersMenuItem);
 contextMenuItems.Add(_groupGetUsersMenuItem);
 contextMenuItems.Add(_groupGetUsersGroupsMenuItem);
 contextMenuItems.Add(_userGetDirectReportsMenuItem);
 contextMenuItems.Add(_userGetGroupsMenuItem);
 contextMenuItems.Add(_userGetSummaryMenuItem);
 break;
 case QueryType.ContextUserDirectReports:
 contextMenuItems.Add(
 _directReportGetDirectReportsMenuItem);
 contextMenuItems.Add(_directReportGetGroupsMenuItem);
 contextMenuItems.Add(_directReportGetSummaryMenuItem);
 contextMenuItems.Add(_userGetGroupsMenuItem);
 contextMenuItems.Add(_userGetSummaryMenuItem);
 break;
 case QueryType.ContextUserGroups:
 contextMenuItems.Add(_groupGetComputersMenuItem);
 contextMenuItems.Add(_groupGetUsersMenuItem);
 contextMenuItems.Add(_groupGetUsersDirectReportsMenuItem);
 contextMenuItems.Add(_groupGetUsersGroupsMenuItem);
 contextMenuItems.Add(_userGetDirectReportsMenuItem);
 contextMenuItems.Add(_userGetSummaryMenuItem);
 break;
 case QueryType.OuComputers:
 contextMenuItems.Add(_computerGetGroupsMenuItem);
 contextMenuItems.Add(_computerGetSummaryMenuItem);
 break;
 case QueryType.OuGroups:
 contextMenuItems.Add(_groupGetComputersMenuItem);
 contextMenuItems.Add(_groupGetUsersMenuItem);
 contextMenuItems.Add(_groupGetUsersDirectReportsMenuItem);
 contextMenuItems.Add(_groupGetUsersGroupsMenuItem);
 contextMenuItems.Add(_groupGetSummaryMenuItem);
 break;
 case QueryType.OuUsers:
 contextMenuItems.Add(_userGetDirectReportsMenuItem);
 contextMenuItems.Add(_userGetGroupsMenuItem);
 contextMenuItems.Add(_userGetSummaryMenuItem);
 break;
 case QueryType.OuUsersDirectReports:
 contextMenuItems.Add(
 _directReportGetDirectReportsMenuItem);
 contextMenuItems.Add(_directReportGetGroupsMenuItem);
 contextMenuItems.Add(_directReportGetSummaryMenuItem);
 contextMenuItems.Add(_userGetDirectReportsMenuItem);
 contextMenuItems.Add(_userGetGroupsMenuItem);
 contextMenuItems.Add(_userGetSummaryMenuItem);
 break;
 case QueryType.OuUsersGroups:
 contextMenuItems.Add(_groupGetComputersMenuItem);
 contextMenuItems.Add(_groupGetUsersMenuItem);
 contextMenuItems.Add(_groupGetUsersDirectReportsMenuItem);
 contextMenuItems.Add(_groupGetUsersGroupsMenuItem);
 contextMenuItems.Add(_userGetDirectReportsMenuItem);
 contextMenuItems.Add(_userGetGroupsMenuItem);
 contextMenuItems.Add(_userGetSummaryMenuItem);
 break;
 case QueryType.ContextComputerSummary:
 contextMenuItems.Add(_computerGetGroupsMenuItem);
 break;
 case QueryType.ContextDirectReportSummary:
 contextMenuItems.Add(
 _directReportGetDirectReportsMenuItem);
 contextMenuItems.Add(_directReportGetGroupsMenuItem);
 break;
 case QueryType.ContextGroupSummary:
 contextMenuItems.Add(_groupGetComputersMenuItem);
 contextMenuItems.Add(_groupGetUsersMenuItem);
 contextMenuItems.Add(_groupGetUsersDirectReportsMenuItem);
 contextMenuItems.Add(_groupGetUsersGroupsMenuItem);
 break;
 case QueryType.ContextUserSummary:
 contextMenuItems.Add(_userGetDirectReportsMenuItem);
 contextMenuItems.Add(_userGetGroupsMenuItem);
 break;
 default:
 throw new ArgumentOutOfRangeException();
 }
 return contextMenuItems;
}

• JanDotNet: I guess that the method is in the same viev model as the methods from your last question!?
• Michael Brandon Morris: @JanDotNet It is (unfortunately). I was unable to get your suggestion for it working, but I did modify the code such that the context menu is now declared in the XAML, and its contents are bound to the property that is set by this method.

Answer 1

I don't think there other good posibilities then using a dictionary instead of a switch;

private List<MenuItem> GetCureentContextMenuItems()
{
 var contextMenus = new Dictionary<QueryType, List<MenuItem>>
 {
 [QueryType.ContextComputerGroups] = new List<MenuItem>
 {
 _groupGetComputersMenuItem,
 _groupGetUsersMenuItem,
 _groupGetUsersDirectReportsMenuItem,
 _groupGetUsersGroupsMenuItem
 },
 [QueryType.ContextDirectReportDirectReports] = new List<MenuItem>
 {
 _directReportGetDirectReportsMenuItem,
 _directReportGetGroupsMenuItem,
 // ...
 },
 // ...
 };
 // if you require it to be a list or you can 
 return contextMenus[QueryType];
}

I would change the name of the method to GetCurrentQueryContextMenuItems as it does do it for the current QueryType and not generating them for an external parameter.

• Xiaoy312: This code doesn't compile. You can't return a IEnumerable<MenuItem> here. You should've used a Dictionary<QueryType, List<MenuItem>>. There also is 2 small details you missed : it throws KeyNotFoundException instead of ArgumentOutOfRangeException and you wrote cureent.
• t3chb0t: @Xiaoy312 thx, notepad coding ;-) fixed. The argument-out-of-range-exeption doesn't make any sense either but you need something there to satisfy the compiler. Besides it's only a sample. You can always try to get the list with the TryGetValue method.