6
\$\begingroup\$

I have a Back function, and it is growing. I also think this is terrible, but I don't seem to get how it can be improved:

public void GoBack()
{
    NavButtonUsed = true;

    Forward.Insert(0, Back[0]);
    Back.RemoveAt(0);

    if (Back[0].Title.StartsWith(" ") && CurrentItem.Menu != Back[0].Menu)
    {
        switch (Back[0].Menu)
        {
            case Menus.WSOneNote:
                CurrentItem = ItemList[0];
                break;

            case Menus.WSMainMenu:
                CurrentItem = ItemList[ItemList.IndexOf(MenuItem.CreateMenuItem(resourceFile.GetString("MainMenu"), typeof(WindowsData.MainMenu), Menus.WSMainMenu))];
                break;

            case Menus.WSTextMenu:
                CurrentItem = ItemList[ItemList.IndexOf(MenuItem.CreateMenuItem(resourceFile.GetString("TextMenu"), typeof(WindowsData.TextMenu), Menus.WSTextMenu))];
                break;

            case Menus.WSTextBlockMenu:
                CurrentItem = ItemList[ItemList.IndexOf(MenuItem.CreateMenuItem(resourceFile.GetString("TextBlockMenu"), typeof(WindowsData.TextBlockMenu), Menus.WSTextBlockMenu))];
                break;

            case Menus.WSTableMenu:
                CurrentItem = ItemList[ItemList.IndexOf(MenuItem.CreateMenuItem(resourceFile.GetString("TableMenu"), typeof(WindowsData.TableMenu), Menus.WSTableMenu))];
                break;

            case Menus.WSTableCellsMenu:
                CurrentItem = ItemList[ItemList.IndexOf(MenuItem.CreateMenuItem(resourceFile.GetString("TableCellsMenu"), typeof(WindowsData.TableCellsMenu), Menus.WSTableCellsMenu))];
                break;

            case Menus.WSDrawMenu:
                CurrentItem = ItemList[ItemList.IndexOf(MenuItem.CreateMenuItem(resourceFile.GetString("DrawMenu"), typeof(WindowsData.DrawMenu), Menus.WSDrawMenu))];
                break;

            case Menus.WSDrawnItemsMenu:
                CurrentItem = ItemList[ItemList.IndexOf(MenuItem.CreateMenuItem(resourceFile.GetString("DrawnItemsMenu"), typeof(WindowsData.DrawnItemsMenu), Menus.WSDrawnItemsMenu))];
                break;

            case Menus.WSPictureMenu:
                CurrentItem = ItemList[ItemList.IndexOf(MenuItem.CreateMenuItem(resourceFile.GetString("PictureMenu"), typeof(WindowsData.PictureMenu), Menus.WSPictureMenu))];
                break;

            case Menus.WSFileMenu:
                CurrentItem = ItemList[ItemList.IndexOf(MenuItem.CreateMenuItem(resourceFile.GetString("FileMenu"), typeof(WindowsData.File), Menus.WSFileMenu))];
                break;

            case Menus.WSAppBarsMenu:
                CurrentItem = ItemList[ItemList.IndexOf(MenuItem.CreateMenuItem(resourceFile.GetString("AppBarsMenu"), typeof(WindowsData.AppBar), Menus.WSAppBarsMenu))];
                break;

            case Menus.WSCharmsMenu:
                CurrentItem = ItemList[ItemList.IndexOf(MenuItem.CreateMenuItem(resourceFile.GetString("CharmsMenu"), typeof(WindowsData.Charms), Menus.WSCharmsMenu))];
                break;

            default:
                break;
        }
    }

    CurrentItem = ItemList[ItemList.IndexOf(Back[0])];
    NavButtonUsed = false;
}

I have more menus to account for, but I haven't implemented going back to them yet as I do not have them fully implemented. What is really going on here is I need to programmatically select a menu to open it so I can select a menu item within it. If I do not open the menu, ItemList will not have CurrentItem, so the app will crash.

This is CurrentItem:

private MenuItem _currentItem = new MenuItem(resourceFile.GetString("OneNote"), typeof(WindowsData.OneNote), Menus.WSOneNote);
public MenuItem CurrentItem
{
    get
    { 
        return _currentItem;
    }
    set
    {
        if (value != _currentItem)
        {
            _currentItem = value;
            OnPropertyChanged();
        }
    }
}

ItemList, Back, and Forward are just ObservableCollections of MenuItems (they can't be Lists because they are bound to XAML objects that need to know when they are updated):

public ObservableCollection<MenuItem> Back { get; set; }
public ObservableCollection<MenuItem> Forward { get; set; }
public ObservableCollection<MenuItem> ItemList { get; set; }

This is NavButtonUsed:

private bool NavButtonUsed { get; set; }

CurrentItem is bound to the selected item in my display ListBox with two-way binding, and the ItemsSource is bound to ItemList. Please note that this is the Windows Store subsection of WPF, so I cannot just use a TreeView - that is why my submenus are prefixed with spaces:

<ListBox Name="Items" Grid.Column="0" Grid.RowSpan="2" ItemsSource="{Binding ItemList}" DisplayMemberPath="Title" SelectionChanged="OnSelectionChanged"
         Tapped="Items_Tapped" Margin="-2,-2,0,-2" Style="{Binding Theme, Converter={StaticResource LBStylePick}}"
         ItemContainerStyle="{Binding Theme, Converter={StaticResource LBIStylePick}}" Padding="0,10" SelectedItem="{Binding CurrentItem, Mode=TwoWay}" />

So, when I press my back button and call GoBack, it sets a flag (boolean value, really) that indicates that I am using a navigation button. Then, it adds the item at Back[0] (the currently selected item, actually), to Forward and removes the item at Back[0] so we don't just go back to the page we are currently on. Next, if the current item's name starts with a space (which provides the indentation needed for submenus) and item's menu is not open, I run the switch block to open the menu:

if (Back[0].Title.StartsWith(" ") && CurrentItem.Menu != Back[0].Menu) { }

In the switch block, I find with menu needs to be opened and assign the main menu item to CurrentItem, which selects the menu through the data binding, which calls the SelectionChanged handler, which opens the menu:

switch (Back[0].Menu)
{
    case Menus.WSOneNote:
        CurrentItem = ItemList[0];
        break;
}

After the if block, I have the code to select the item that the user wants to go back to and set the navigation flag to false:

CurrentItem = ItemList[ItemList.IndexOf(Back[0])];
NavButtonUsed = false;

This is the skeleton code for SelectionChanged for reference. This is not the main focus of this post, but this also has a similar problem, so suggestions to solve this also are welcome - preferably if they kill both with one stone:

public void SelectionChanged(MenuItem newSelection)
{
    if (!NavButtonUsed) { Back.Insert(0, newSelection); }

    if (Forward.Count != 0 && !NavButtonUsed) { Forward.Clear(); }

    if (newSelection.Title.StartsWith(" ")) { return; }

    // Remove menu items to close menu - title starts with space
    ItemList.RemoveAll(item => item.Title.StartsWith(" "));

    switch (newSelection.Menu)
    {
        case Menus.WSOneNote:
            ItemList.Insert(1, MenuItem.CreateMenuItem(resourceFile.GetString("WSNotes"), typeof(WindowsData.Submenus.Notes), Menus.WSOneNote));
            ItemList.Insert(1, MenuItem.CreateMenuItem(resourceFile.GetString("WSPages"), typeof(WindowsData.Submenus.Pages), Menus.WSOneNote));
            ItemList.Insert(1, MenuItem.CreateMenuItem(resourceFile.GetString("WSSections"), typeof(WindowsData.Submenus.Section), Menus.WSOneNote));
            ItemList.Insert(1, MenuItem.CreateMenuItem(resourceFile.GetString("WSNotebooks"), typeof(WindowsData.Submenus.Notebook), Menus.WSOneNote));
            break;

        case Menus.WSMainMenu:
            ItemList.Insert(2, MenuItem.CreateMenuItem(resourceFile.GetString("WSDraw"), typeof(WindowsData.Submenus.Draw), Menus.WSMainMenu));
            ItemList.Insert(2, MenuItem.CreateMenuItem(resourceFile.GetString("WSUndo"), typeof(WindowsData.Submenus.Undo), Menus.WSMainMenu));
            ItemList.Insert(2, MenuItem.CreateMenuItem(resourceFile.GetString("WSTag"), typeof(WindowsData.Submenus.Tag), Menus.WSMainMenu));
            ItemList.Insert(2, MenuItem.CreateMenuItem(resourceFile.GetString("WSPaste"), typeof(WindowsData.Submenus.Paste), Menus.WSMainMenu));
            ItemList.Insert(2, MenuItem.CreateMenuItem(resourceFile.GetString("WSList"), typeof(WindowsData.Submenus.List), Menus.WSMainMenu));
            ItemList.Insert(2, MenuItem.CreateMenuItem(resourceFile.GetString("WSTable"), typeof(WindowsData.Submenus.Table), Menus.WSMainMenu));
            ItemList.Insert(2, MenuItem.CreateMenuItem(resourceFile.GetString("WSInsertFile"), typeof(WindowsData.Submenus.FileInsert), Menus.WSMainMenu));
            ItemList.Insert(2, MenuItem.CreateMenuItem(resourceFile.GetString("WSPicture"), typeof(WindowsData.Submenus.Picture), Menus.WSMainMenu));
            break;

        /* ... */
    }
}
\$\endgroup\$
  • \$\begingroup\$ A dictionary might be a good replacement for switch...case. See stackoverflow.com/a/4233539/2101909 \$\endgroup\$ – ben rudgers Jan 28 '15 at 19:12
  • \$\begingroup\$ So I create a dictionary with a Key of Menus.(menu), and a Value of the corresponding MenuItem? Then search it for the corresponding Key corresponding to the Menus.(menu) in Back[0]? \$\endgroup\$ – Hosch250 Jan 28 '15 at 19:16
  • \$\begingroup\$ To be honest, I don't understand your code well enough to be that specific. The key needs to be whatever type the switch takes and the value needs to evaluate to what switch returns. The big idea is that the value can even be a function (anonymous or not). Using a dictionary makes the call site simpler and allows changes in the dispatch without modifying the source for GoBack() once it gets accessor methods for the dictionary. Essentially dictionary use allows a data structure to replace a cumbersome procedure. \$\endgroup\$ – ben rudgers Jan 28 '15 at 19:26
5
\$\begingroup\$

After Ben Rudger's suggestion to use a Dictionary, I realized that if the menu is closed, there is only one instance of a MenuItem in ItemList with the matching Menus.Value. Because of this, all I have to do is iterate over ItemList until I find that matching value, choose that value, and exit the loop, so GoBack becomes this:

public void GoBack()
{
    NavButtonUsed = true;

    Forward.Insert(0, Back[0]);
    Back.RemoveAt(0);

    if (Back[0].Title.StartsWith(" ") && CurrentItem.Menu != Back[0].Menu)
    {
        foreach(MenuItem mi in ItemList)
        {
            if (mi.Menu == Back[0].Menu)
            {
                CurrentItem = ItemList[ItemList.IndexOf(mi)];
                break;
            }
        }
    }

    CurrentItem = ItemList[ItemList.IndexOf(Back[0])];
    NavButtonUsed = false;
}

If the menu is open, we do not use the loop, but it still would work because the menu is still the first instance of MenuItem with its specific Menus.Value in the list.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.