Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Convert menu structure into data #5752

Open
bclothier opened this issue Apr 29, 2021 · 1 comment
Open

Convert menu structure into data #5752

bclothier opened this issue Apr 29, 2021 · 1 comment

Comments

@bclothier
Copy link
Contributor

This arose from a tangential comment in the Peek Definition PR.

Currently, we make use of enum to describe how to layout our menus. This is less than optimal for at least 3 reasons:

  1. We're basically duplicating the names; as a member of an enum and as a menu command. This kind of violates the DRY principles and also thwarts any compile-time checks for the consistency of the menu structure.

  2. The menu layout is technically more data than code.

  3. We are mixing concerns --- we describe the layout in an enum but then we describe the BeginGroup as an override in the corresponding command.

This was developed with a earlier version of C#, and we need to work with commandbar system (blargh) so we can't use XAML or something similar.

However, with C# now featuring nameof() operator, we could consider converting the enum into a static class. Something like this:

public static class MenuLayout
{
   private static readonly List<string> _list => new List<string> 
    {
      nameof(OneCommand),
      nameof(AnotherCommand),
      default, //indicates a begin group?
      nameof(MoreCommand)
     }

   public static string GetElement(int index) => _list.Index(index);
}

The advantage gained is that the nameof operator affords us the type safety and compile-time validation and ability to refactor the command's name without breaking the menu layout.

Note: I suppose it could be a List<Command> instead of List<string> but I'm not 100% sure if there's a need to make it stringified or something. I also am not 100% sold on inserting default as a stand-in for inserting a separator (e.g. BeginGroup).

Would that help improvement our management of the menu layout?

Originally posted by @bclothier in #5751 (comment)

@mansellan
Copy link
Member

Ah yes, I recall not liking this when I did the VB6 work.... Glad it's not just me!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants