Display submenus

Question

I am displaying menus and submenus in my WinRT app, and I think I am doing it wrong. I have thought I was doing it wrong for a long time, so here it is for review.

This is the section in my ViewModel. Probably needless to say, both Remove() and NewSelectionWS() have had large quantities of nearly identical code removed; feel free to comment on everything:

private ObservableCollection<string> _itemList = new ObservableCollection<string>();
public ObservableCollection<string> ItemList
{
    get { return _itemList; }
    set
    {
        _itemList = value;
        OnPropertyChanged("ItemList");
    }
}

private void Remove()
{
    foreach (string s in GlobalVars.OneNote()) { ItemList.Remove(s); }
    foreach (string s in GlobalVars.MainMenu()) { ItemList.Remove(s); }

    foreach (Type t in GlobalVars.OneNoteP()) { Pages.Remove(t); }
    foreach (Type t in GlobalVars.MainMenuP()) { Pages.Remove(t); }
}

public void NewSelectionWS(string val)
{
    if (!val.StartsWith(" ")) { Remove(); }

    switch (val)
    {
        case "Menu 1":
            foreach (string s in GlobalVars.OneNote()) { ItemList.Insert(ItemList.IndexOf("Menu 1") + 1, s); }
            foreach (Type t in GlobalVars.OneNoteP()) { Pages.Insert(ItemList.IndexOf("Menu 1") + 1, t); }
            break;

        case "Menu 2":
            foreach (string s in GlobalVars.MainMenu()) { ItemList.Insert(ItemList.IndexOf("Menu 2") + 1, s); }
            foreach (Type t in GlobalVars.MainMenuP()) { Pages.Insert(ItemList.IndexOf("Menu 2") + 1, t); }
            break;
    }
}

This is GlobalVars:

private static string[] _homeWS = { "Menu 1", "Menu 2", "Menu 3", "Menu 4" };
private static string[] _oneNote = { "   Submenu 1", "   Submenu 2", "   Submenu 3" };
private static string[] _mainMenu = { "   Submenu 1", "   Submenu 2", "   Submenu 3" };

public static string[] HomeWS() { return _homeWS; };
public static string[] OneNote() { return _oneNote; }
public static string[] MainMenu() { return _mainMenu; }

private static Type[] _wsHomeP = { typeof(WindowsData.Menu1), typeof(WindowsData.Menu2), typeof(WindowsData.Menu3), typeof(WindowsData.Menu4) };
private static Type[] _oneNoteP = { typeof(WindowsData.Submenus.Submenu1), typeof(WindowsData.Submenus.Submenu2), typeof(WindowsData.Submenus.Submenu3) };
private static Type[] _mainMenuP = { typeof(WindowsData.Submenus.Submenu1), typeof(WindowsData.Submenus.Submenu2), typeof(WindowsData.Submenus.Submenu3) };

public static Type[] HomeP() {  return _wsHomeP; }
public static Type[] OneNoteP() { return _oneNoteP; }
public static Type[] MainMenuP() { return _mainMenuP; }

This is the relevant XAML in MainPage.xaml:

<ListBox Name="Items" Grid.Column="0" Grid.RowSpan="2" ItemsSource="{Binding ItemList}" />

<Frame Grid.Column="1" Grid.Row="1" Name="DataFrame" FontSize="{Binding SetFontSize}" VerticalAlignment="Top" />

This is the code-behind for a selection in MainPage.xaml.cs:

public static ViewModel Data = new ViewModel();

private void NewSelect(object sender, SelectionChangedEventArgs e)
{
    switch(Data.OneNoteVersion)
    {
        case 0:
            Data.NewSelectionWS(Items.SelectedValue.ToString());
            break;
        case 1:
            Data.NewSelectionP(Items.SelectedValue.ToString());
            break;
        case 2:
            Data.NewSelection2013(Items.SelectedValue.ToString());
            break;
    }

    DataFrame.Navigate(Data.Pages[Items.SelectedIndex]);
}

I like the way it is displaying, but I doubt it is correct to use spaces to create submenus. I also am doubtful about my removing and entered the submenus the way I do. I have looked into a couple different methods, but nothing really seemed to suit my needs; I will appreciate any input. I will also appreciate comments on my use of GlobalVars; I think this is the best way to hard-code data, but I am not quite sure if there is a better way; the default apps use JSON files, but I don't know how to use them. Is this something I should learn and implement, or is it fine the way it is?

Screenshot of what I am doing:

Answer 1

Ok now I see the whole picture. So my comment to your code is that your idea to workaround lack of TreeView (which, by the way, can be customized to be always expanded and without +- icons) is pretty good, but the way you approach is not. Generally what you are doing wrong is that you mix menu logic with UI. To be concrete:

1. Create a class that represents a menu item, give it properties like name (without spaces), depth level, add a collection of children menu items. Make the class inherit form INotifyPropertyChanged interface to provide fully-capable view model class (or just skip it if you don't want to modyfi your menus at runtime or replace always whole tree). Don't put any UI code in this view model.

2. Create user control with dependency property that you bind to root menu. In the code behind fill your list view depending on the view model, calculate number of spaced dynamically depending on menu item depth.

This'll make your solution very robust and extendable. By adding new properties to menu item view model you'll be able to easily customize each item.

Answer 2

private void Remove()

Remove... Remove what? How about RemovePages? Even that wouldn't be a great name though, because the method seems to be doing two different things. (This could be my own lack of understanding, perhaps you have a good reason. Maybe these lists need to be kept in sync. If that's the case, you're probably using the wrong data structure for the job.)

private void Remove()
{
    foreach (string s in GlobalVars.OneNote()) { ItemList.Remove(s); }
    foreach (string s in GlobalVars.MainMenu()) { ItemList.Remove(s); }

    foreach (Type t in GlobalVars.OneNoteP()) { Pages.Remove(t); }
    foreach (Type t in GlobalVars.MainMenuP()) { Pages.Remove(t); }
}

As you noted, there's some duplication here. Minimally, you can extract two methods.

private void RemoveFromItemList(string[] items)
{
    foreach (var s in items)
    {
        ItemList.Remove(s);
    }
}

private void RemoveFromPages(Type types)
{
    foreach(var t in types)
    {
        Pages.Remove(t);
    }
 }

This doesn't buy you much though. Just the ability to format your code nicely without making Remove twenty lines long.

Now, the more I look at your GlobalVars, the more I don't like this parallel array thing you have going on. I would create a simple struct (or perhaps a class) to hold this information in. Something along these lines.

public struct MenuItem
{
    public string Name;
    public Type Type;
}

And then keep a single array of these, instead of two parallel arrays of strings and types.