5
\$\begingroup\$

So, this is my refactored VM:

public partial class MainPageVM : INotifyPropertyChanged
{
    private IThemeProvider _ThemeProvider;
    private IStartupVersionProvider _StartupVersionProvider;
    private IFontSizeProvider _FontSizeProvider;

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

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

    private ObservableCollection<MenuItem> _searchList = new ObservableCollection<MenuItem>();
    public ObservableCollection<MenuItem> SearchList
    {
        get { return _searchList; }
        set
        {
            if (_searchList == value) return;
            _searchList = value;
            OnPropertyChanged();
        }
    }

    private int _theme = 0;
    public int Theme
    {
        get { return _theme; }
        set
        {
            if (value == _theme) return;
            _theme = value;
            OnPropertyChanged();
        }
    }

    private int _fontSize = 20;
    public int SetFontSize
    {
        get { return _fontSize; }
        set
        {
            if (value == _fontSize) return;
            _fontSize = value;
            OnPropertyChanged();
        }
    }

    private OneNoteVersionEnum _startupVersion = OneNoteVersionEnum.WStore;
    public OneNoteVersionEnum StartupVersion
    {
        get { return _startupVersion; }
        set
        {
            if (_startupVersion == value) return;
            _startupVersion = value;
            OnPropertyChanged();
        }
    }

    private OneNoteVersionEnum _OneNoteVersion = OneNoteVersionEnum.WStore;
    public OneNoteVersionEnum OneNoteVersion
    {
        get { return _OneNoteVersion; }
        set
        {
            if (_OneNoteVersion == value) return;
            _OneNoteVersion = value;
            OnPropertyChanged();
        }
    }

    public event PropertyChangedEventHandler PropertyChanged;
    protected virtual void OnPropertyChanged([CallerMemberName]string propertyName = null)
    {
        PropertyChangedEventHandler handler = PropertyChanged;
        if (handler != null)
        {
            handler(this, new PropertyChangedEventArgs(propertyName));
        }
    }

    public static ResourceLoader resourceFile = new ResourceLoader();

    public MainPageVM(IThemeProvider themeProvider, IStartupVersionProvider startupVersionProvider, IFontSizeProvider fontSizeProvider)
    {
        _ThemeProvider = themeProvider;
        _StartupVersionProvider = startupVersionProvider;
        _FontSizeProvider = fontSizeProvider;

        Theme = _ThemeProvider.GetCurrentTheme();
        SetFontSize = _FontSizeProvider.GetCurrentFontSize();
        StartupVersion = (OneNoteVersionEnum)_StartupVersionProvider.GetCurrentStartupVersion();

        Back = new ObservableCollection<MenuItem>();
        Forward = new ObservableCollection<MenuItem>();
        ItemList = new ObservableCollection<MenuItem>();
        NavButtonUsed = false;

        OneNoteVersion = StartupVersion;
        SwitchTutorial(OneNoteVersion);

        CurrentItem = ItemList[0];
    }

    public void InsertItemListItem(int index, string title, Type page, Menus menu)
    {
        ItemList.Insert(index, new MenuItem(resourceFile.GetString(title), page, menu));
    }

    public void SelectionChangedWS()
    {
        if (!NavButtonUsed) { Back.Insert(0, CurrentItem); }

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

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

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

        switch (CurrentItem.Menu)
        {
            // I insert these at 1 because 0 is the index of the menu, and I inserted them backwards so I could insert them all in the same index to better represent they are all part of the same menu.
            case Menus.WSOneNote:
                InsertItemListItem(1, "WSNotes", typeof(Tutorials.WindowsData.Submenus.Notes), Menus.WSOneNote);
                InsertItemListItem(1, "WSPages", typeof(Tutorials.WindowsData.Submenus.Pages), Menus.WSOneNote);
                InsertItemListItem(1, "WSSections", typeof(Tutorials.WindowsData.Submenus.Section), Menus.WSOneNote);
                InsertItemListItem(1, "WSNotebooks", typeof(Tutorials.WindowsData.Submenus.Notebook), Menus.WSOneNote);
                break;

            case Menus.WSMainMenu:
                InsertItemListItem(2, "WSDraw", typeof(Tutorials.WindowsData.Submenus.Draw), Menus.WSMainMenu);
                InsertItemListItem(2, "WSUndo", typeof(Tutorials.WindowsData.Submenus.Undo), Menus.WSMainMenu);
                InsertItemListItem(2, "WSTag", typeof(Tutorials.WindowsData.Submenus.Tag), Menus.WSMainMenu);
                InsertItemListItem(2, "WSPaste", typeof(Tutorials.WindowsData.Submenus.Paste), Menus.WSMainMenu);
                InsertItemListItem(2, "WSList", typeof(Tutorials.WindowsData.Submenus.List), Menus.WSMainMenu);
                InsertItemListItem(2, "WSTable", typeof(Tutorials.WindowsData.Submenus.Table), Menus.WSMainMenu);
                InsertItemListItem(2, "WSInsertFile", typeof(Tutorials.WindowsData.Submenus.FileInsert), Menus.WSMainMenu);
                InsertItemListItem(2, "WSPicture", typeof(Tutorials.WindowsData.Submenus.Picture), Menus.WSMainMenu);
                break;

            /* ... */

            default:
                break;
        }
    }

    public void SelectionChanged2013()
    {
        if (!NavButtonUsed) { Back.Insert(0, CurrentItem); }

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

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

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

        switch (CurrentItem.Menu)
        {
            /* Same things as above */
        }
    }

    public void GoBack()
    {
        NavButtonUsed = true;

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

        if (Back[0].Title.StartsWith(" ") && CurrentItem.Menu != Back[0].Menu)
        {
            CurrentItem = ItemList.FirstOrDefault(i => i.Menu == Back[0].Menu);
        }

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

    public void GoForward()
    {
        NavButtonUsed = true;
        CurrentItem = ItemList[ItemList.IndexOf(Forward[0])];
        Back.Insert(0, Forward[0]);
        Forward.RemoveAt(0);
        NavButtonUsed = false;
    }

    public void SwitchTutorial(OneNoteVersionEnum version)
    {
        if (ItemList.Count != 0) { CurrentItem = ItemList[0]; }
        ItemList.RemoveAll(item => item.Title != resourceFile.GetString("OneNote"));

        Back.Clear();
        Forward.Clear();

        if (version == OneNoteVersionEnum.WStore)
        {
            OneNoteVersion = OneNoteVersionEnum.WStore;

            InsertItemListItem(0, "OneNote", typeof(Tutorials.WindowsData.OneNote), Menus.WSOneNote);
            ItemList.Add(new MenuItem(resourceFile.GetString("WSMainMenu"), typeof(Tutorials.WindowsData.MainMenu), Menus.WSMainMenu));
            ItemList.Add(new MenuItem(resourceFile.GetString("WSTextMenu"), typeof(Tutorials.WindowsData.TextMenu), Menus.WSTextMenu));
            ItemList.Add(new MenuItem(resourceFile.GetString("WSTextBlockMenu"), typeof(Tutorials.WindowsData.TextBlockMenu), Menus.WSTextBlockMenu));
            ItemList.Add(new MenuItem(resourceFile.GetString("WSTableMenu"), typeof(Tutorials.WindowsData.TableMenu), Menus.WSTableMenu));
            ItemList.Add(new MenuItem(resourceFile.GetString("WSTableCellsMenu"), typeof(Tutorials.WindowsData.TableCellsMenu), Menus.WSTableCellsMenu));
            ItemList.Add(new MenuItem(resourceFile.GetString("WSDrawMenu"), typeof(Tutorials.WindowsData.DrawMenu), Menus.WSDrawMenu));
            ItemList.Add(new MenuItem(resourceFile.GetString("WSDrawnItemsMenu"), typeof(Tutorials.WindowsData.DrawnItemsMenu), Menus.WSDrawnItemsMenu));
            ItemList.Add(new MenuItem(resourceFile.GetString("WSPictureMenu"), typeof(Tutorials.WindowsData.PictureMenu), Menus.WSPictureMenu));
            ItemList.Add(new MenuItem(resourceFile.GetString("WSFileMenu"), typeof(Tutorials.WindowsData.File), Menus.WSFileMenu));
            ItemList.Add(new MenuItem(resourceFile.GetString("WSAppBarsMenu"), typeof(Tutorials.WindowsData.AppBar), Menus.WSAppBarsMenu));
            ItemList.Add(new MenuItem(resourceFile.GetString("WSCharmsMenu"), typeof(Tutorials.WindowsData.Charms), Menus.WSCharmsMenu));
            ItemList.Add(new MenuItem(resourceFile.GetString("WSKeyboardShortcutsMenu"), typeof(Tutorials.WindowsData.KeyboardShortcuts), Menus.WSKeyboardShortcuts));
            ItemList.Add(new MenuItem(resourceFile.GetString("WSColorPalateMenu"), typeof(Tutorials.WindowsData.ColorPalate), Menus.WSColorPalate));

            CurrentItem = ItemList[0];
            if (ItemList[5].Title == resourceFile.GetString("OneNote")) { ItemList.RemoveAt(5); }
        }

        if (version == OneNoteVersionEnum.WPhone)
        {
            OneNoteVersion = OneNoteVersionEnum.WPhone;

            InsertItemListItem(0, "OneNote", typeof(Tutorials.PhoneData.OneNote), Menus.WPOneNote);
            ItemList.Add(new MenuItem(resourceFile.GetString("WPNotebooks"), typeof(Tutorials.PhoneData.Submenus.Notebook), Menus.WPOneNote));
            ItemList.Add(new MenuItem(resourceFile.GetString("WPSections"), typeof(Tutorials.PhoneData.Submenus.Sections), Menus.WPOneNote));
            ItemList.Add(new MenuItem(resourceFile.GetString("WPPages"), typeof(Tutorials.PhoneData.Submenus.Pages), Menus.WPOneNote));
            ItemList.Add(new MenuItem(resourceFile.GetString("WPNotes"), typeof(Tutorials.PhoneData.Submenus.Notes), Menus.WPOneNote));
            ItemList.Add(new MenuItem(resourceFile.GetString("WPCortana"), typeof(Tutorials.PhoneData.Submenus.Cortana), Menus.WPOneNote));

            CurrentItem = ItemList[0];
            if (ItemList[1].Title == resourceFile.GetString("OneNote")) { ItemList.RemoveAt(1); }
        }

        if (version == OneNoteVersionEnum.Desktop2013)
        {
            OneNoteVersion = OneNoteVersionEnum.Desktop2013;

            InsertItemListItem(0, "OneNote", typeof(Tutorials.Desktop2013Data.Menus.OneNote), Menus.OneNote2013);
            ItemList.Add(new MenuItem(resourceFile.GetString("File2013"), typeof(Tutorials.Desktop2013Data.Menus.FileTab), Menus.File2013));
            ItemList.Add(new MenuItem(resourceFile.GetString("Home2013"), typeof(Tutorials.Desktop2013Data.Menus.HomeTab), Menus.Home2013));
            ItemList.Add(new MenuItem(resourceFile.GetString("Insert2013"), typeof(Tutorials.Desktop2013Data.Menus.InsertTab), Menus.Insert2013));
            ItemList.Add(new MenuItem(resourceFile.GetString("Draw2013"), typeof(Tutorials.Desktop2013Data.Menus.DrawTab), Menus.Draw2013));
            ItemList.Add(new MenuItem(resourceFile.GetString("History2013"), typeof(Tutorials.Desktop2013Data.Menus.HistoryTab), Menus.History2013));
            ItemList.Add(new MenuItem(resourceFile.GetString("Review2013"), typeof(Tutorials.Desktop2013Data.Menus.ReviewTab), Menus.Review2013));
            ItemList.Add(new MenuItem(resourceFile.GetString("View2013"), typeof(Tutorials.Desktop2013Data.Menus.ViewTab), Menus.View2013));
            ItemList.Add(new MenuItem(resourceFile.GetString("Playback2013"), typeof(Tutorials.Desktop2013Data.Menus.PlaybackTab), Menus.Playback2013));
            ItemList.Add(new MenuItem(resourceFile.GetString("Layout2013"), typeof(Tutorials.Desktop2013Data.Menus.LayoutTab), Menus.Layout2013));
            ItemList.Add(new MenuItem(resourceFile.GetString("Design2013"), typeof(Tutorials.Desktop2013Data.Menus.DesignTab), Menus.Design2013));
            ItemList.Add(new MenuItem(resourceFile.GetString("Share2013"), typeof(Tutorials.Desktop2013Data.Menus.ShareTab), Menus.Share2013));

            CurrentItem = ItemList[0];
            if (ItemList[1].Title == resourceFile.GetString("OneNote")) { ItemList.RemoveAt(1); }
        }

        OneNoteVersion = version;
    }

    public void SearchAndSort(ref string[] query)
    {
        List<int> weight = new List<int>();

        int position = -1;

        foreach (KeyValuePair<string[], MenuItem> val in SearchKeys.SearchData)
        {
            position++;
            int length = val.Key.Length;
            int middle = length / 2;
            char firstCharMidArray = val.Key[middle][0];

            foreach (string s in query)
            {
                if (s.Length == 0) continue;

                int min = val.Key[middle][0] < s[0] ? middle : 0;
                int max = val.Key[middle][0] <= s[0] ? val.Key.Length : middle + 1;

                for (int i = min; i < max; i++)
                {
                    weight.Add(0);

                    if (val.Key[i] == s)
                    {
                        if (weight[position] == 0)
                        {
                            SearchList.Add(val.Value);
                        }
                        weight[position]++;
                    }
                }
            }
        }

        SortResults(ref weight);
    }

    private void SortResults(ref List<int> weights)
    {
        while (weights.Contains(0)) weights.Remove(0);

        ObservableCollection<MenuItem> localValues = new ObservableCollection<MenuItem>();
        List<int> localWeights = new List<int>();

        int index = -1;
        var weightsWithIndex = weights.Select(p => new { Value = p, Index = ++index }).OrderByDescending(p => p.Value);

        foreach (var w in weightsWithIndex)
        {
            localWeights.Add(w.Value);
            localValues.Add(SearchList[w.Index]);
        }

        SearchList = localValues;
        weights = localWeights;
    }
}

This is some of the supporting material used by the VM:

public enum OneNoteVersionEnum
{
    WStore, WPhone, Desktop2013
}

public enum Menus
{
    WSOneNote, WSMainMenu, WSTextMenu, WSTextBlockMenu, WSTableMenu, WSTableCellsMenu,
    WSDrawMenu, WSDrawnItemsMenu, WSPictureMenu, WSFileMenu, WSAppBarsMenu, WSCharmsMenu,
    WSKeyboardShortcuts, WSColorPalate,

    WPOneNote,

    OneNote2013, File2013, Home2013, Insert2013, Draw2013, History2013, Review2013, View2013,
    Playback2013, Layout2013, Design2013, Share2013,

    SearchResult
}

public class MenuItem
{
    public MenuItem(string title, Type page, Menus menu)
    {
        Title = title;
        Page = page;
        Menu = menu;
    }

    public override bool Equals(object obj)
    {
        MenuItem menu = (MenuItem)obj;
        return menu.Title == Title && menu.Page == Page && menu.Menu == Menu;
    }

    private string _title = "";
    public string Title
    {
        get { return _title; }
        set { _title = value; }
    }

    private Type _page = null;
    public Type Page
    {
        get { return _page; }
        set { _page = value; }
    }

    private Menus _menu = new Menus();
    public Menus Menu
    {
        get { return _menu; }
        set { _menu = value; }
    }
}

And this is a quick example of how it is used:

<AppBar Height="100" VerticalAlignment="Top">
    <StackPanel Orientation="Horizontal">
        <Button Name="StoreOneNote" Margin="10" Style="{StaticResource ABButton}" Content="Windows&#10;Store" Click="Store_Click" VerticalAlignment="Top"/>
        <Button Name="PhoneOneNote" Margin="10" Style="{StaticResource ABButton}" Content="Windows&#10;Phone" Click="Phone_Click" VerticalAlignment="Top"/>
        <Button Name="OneNote2013Button" Margin="10" Style="{StaticResource ABButton}" Content="OneNote 2013" Click="OneNote2013_Click" VerticalAlignment="Top"/>
    </StackPanel>
</AppBar>

<AppBar Grid.ColumnSpan="2" Grid.RowSpan="2" VerticalAlignment="Bottom" Padding="0,0,10,0">
    <CommandBar>
        <CommandBar.SecondaryCommands>
            <AppBarButton Icon="Back" Label="Back" Click="Back_Click" IsEnabled="{Binding Back.Count, Converter={StaticResource BackToBool}}"/>
            <AppBarButton Icon="Forward" Label="Forward" Click="Forward_Click" IsEnabled="{Binding Forward.Count, Converter={StaticResource ForwardToBool}}"/>
        </CommandBar.SecondaryCommands>
</AppBar>
private void Back_Click(object sender, RoutedEventArgs e)
{
    Data.GoBack();
}

private void Forward_Click(object sender, RoutedEventArgs e)
{
    Data.GoForward();
}

private void Store_Click(object sender, RoutedEventArgs e)
{
    Data.SwitchTutorial(OneNoteVersionEnum.WStore);
}

private void Phone_Click(object sender, RoutedEventArgs e)
{
    Data.SwitchTutorial(OneNoteVersionEnum.WPhone);
}

private void OneNote2013_Click(object sender, RoutedEventArgs e)
{
    Data.SwitchTutorial(OneNoteVersionEnum.Desktop2013);
}

If you need any more information, please let me know. I removed some very repetitive code to shorten my question so it would fit better and be more easily reviewable, but the gist of it is the same. What I am mainly looking for is whether I am using the MVVM model correctly, but I am sure there are things that can be improved as well.

\$\endgroup\$
4
\$\begingroup\$

What I don't like about your design is, that you are creating and removing MenuItems all the time.

You should consider to create a new class MenuItemManager which is loading all the MenuItem's at the start and then providing properties or methods to retrieve the loaded MenuItem's.

You should consider to change the OneNoteVersionEnum enum to AppVersion or ApplicationVersion to keep the possibility of extending your application to take care of other applications too.

MenuItem

Calling Equals() with an object which isn't a MenuItem will result in an InvalidCastException.

If Equals() is overridden it is mandatory (at least if you use a Dictionary or HashMap) or at least recommended to override GetHashCode() too.

You should add a property for holding the mentioned AppVersion for filtering. Using auto properties with public getters and private setters will reduce the amount of code.

public class MenuItem
{
    public string Title { get; private set; }
    public Type Page { get; private set; }
    public Menus Menu { get; private set; }
    public AppVersion Version { get; private set; }

    public MenuItem(string title, Type page, Menus menu, AppVersion appVersion)
    {
        Title = title;
        Page = page;
        Menu = menu;
        Version = appVersion;
    }

    public override bool Equals(object obj)
    {
        MenuItem test = (MenuItem)obj;

        MenuItem other = obj as MenuItem;
        if (other == null) { return false; }

        return other.Title == this.Title &&
               other.Page == this.Page &&
               other.Menu == this.Menu &&
               other.Version == this.Version;

    }

    public override int GetHashCode()
    {
        unchecked
        {
            var hash = 13;
            hash = (hash * 7) ^ this.Title.GetHashCode();
            hash = (hash * 7) ^ this.Page.GetHashCode();
            hash = (hash * 7) ^ this.Menu.GetHashCode();
            hash = (hash * 7) ^ this.Version.GetHashCode();
            return hash;
        }
    }
}  

with

public enum AppVersion
{
    OneNoteWStore, OneNoteWPhone, Desktop2013
}  

To manage your MenuItem's, we add a MenuItemManager class which contains methods to fill and retrieve the items

public class MenuItemManager
{
    private List<MenuItem> menuItems = new List<MenuItem>();

    public MenuItem this[String title]
    {
        get
        {
            return menuItems.Find(m => m.Title == title);
        }
    }

    public IEnumerable<MenuItem> GetMenuItems(AppVersion version)
    {
        return menuItems.Where(m => m.Version == version);
    }

    public IEnumerable<MenuItem> GetMenuItems(Menus menu)
    {
        return menuItems.Where(m => m.Menu == menu);
    }

    public MenuItemManager()
    {
        Fill();
    }

    private void Fill()
    {
        //use this to fill the meniItems  
        menuItems.Add(new MenuItem(resourceFile.GetString("WPCortana"), typeof(Tutorials.PhoneData.Submenus.Cortana), Menus.WPOneNote));

        //...
    }
}

which can be extended to retrieve MenuItem(s) based on different criteria.

So e.g the former SelectionChangedWS() method could be simplified to

MenuItemManager manager = new MenuItemManager();
public void SelectionChangedWS()
{
    if (!NavButtonUsed) { Back.Insert(0, CurrentItem); }

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

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

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

    foreach (MenuItem item in manager.GetMenuItems(CurrentItem.Menu))
    {
        ItemList.Insert(1, item);
    }

}

which leads to the possibility to combine SelectionChanged2013() and SelectionChangedWS() into one method.

\$\endgroup\$
1
  • \$\begingroup\$ Great points - especially about the MenuItemManager - I really hated that, and wasn't sure how to fix it. \$\endgroup\$
    – user34073
    Feb 10 '15 at 16:24
2
\$\begingroup\$

Just to get some of this cleared out of the way, I realized I can change MenuItem to this simpler version:

public string Title
{
    get;
    set;
}

public Type Page
{
    get;
    set;
}

public Menus Menu
{
    get;
    set;
}

Also, I should merge SelectionChangedWS and SelectionChanged2013 as they are doing the same thing, although it will become a monster method that way. If I really want to keep them separate, I should at least merge them and only put the switch statements in their own methods.

\$\endgroup\$

Your Answer

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