Lambda .Where() expression to return selected/flagged objects

Question

In our software we enable certain features based on the existence of a flag. The Flags class is a static class used to determine the existence of such a flag.

I came across one piece of logic that I wrote that I feel could benefit from refactoring. The boolean logic works, but I think it's difficult to understand or read because it relies very heavily on short-circuiting.

The basic logic is:

1. Get all menu items.
2. Return a subset of those menu items by using the following rule:
   • Some menu items are tied to a specific flag that is expected to be hard-coded in some way. The menu item name may not match the associated flag.
   • Any other menu items should be displayed.

Any help would be much appreciated.

private IEnumerable<MenuItemDefinition> GetMenuItems()
{
 return GetAllMenuItems().Where(mi => 
 (mi.MethodName != "NewWidget" || Flags.HasFlag("Feature1")) &&
 (mi.MethodName != "NewDongle" || Flags.HasFlag("Feature2")));
}

Answer 1

If you control the MenuItemDefinition class, why not just add a feature tag to it?

public class MenuItemDefinition
{
 public string MethodName { get; set; }
 public string Feature { get; set; }
}
// each instance
new MenuItemDefinition { MethodName = "NewWidget", Feature = "Widgets" }
// GetMenuItems():
return GetAllMenuItems().Where(m => FeatureIsEnabled(m.Feature));

If not, you could put them in a dictionary:

var items = new Dictionary<string, MenuItemDefinition[]> {
 { "Widgets", new[] {
 new MenuItemDefinition { MethodName = "NewWidget" },
 new MenuItemDefinition { MethodName = "DeleteWidget" }
 },
 { "Default", new[] {
 // ....
 }
}
return items.Where(pair => FeatureIsEnabled(pair.Key)).SelectMany(pair => pair.Value);

Or better yet, build the initial list based upon content in a dictionary. That way you don't run the risk of having typos in the switch or the initial setup:

var dict = new Dictionary<string, string[]> 
{
 { "Widgets", new[] { "NewWidget", "DeleteWidget" },
 { "Default", new[] { "..." }
 ...
}
var menuItems = dict
 .Where(p => FeatureIsEnabled(p.Key))
 .SelectMany(p => p.Value)
 .Select(mn => new MenuItemDefinition { MethodName = mn });
// GetMenuItems()
return menuItems; // :)

• Kevin McCormick: Hi @Lars-Erik, thanks for the answer. I've provided additional information in the example that answers your question, I hope that is helpful.
• Lars-Erik: I got it, entirely new answer. ;)
• Kevin McCormick: Thanks for the updated response. I'm not able to make changes to the MenuItemDefinition class, but if I were able to, this would be the best solution. For reference, I've posted my solution separately as well.
• Lars-Erik: Oh no, not the switch! It's evil. ;) At least pull it out and hide it in a method as I first described. Anyway, I added a third proposal, which I hope appeals.
• Kevin McCormick: Ah yes, switch is very evil indeed! The GetMenuItems() method is in fact the method that hides the implementation so we are already in a hidden place for such evil things. That being said, I like the dictionary approach even more!