HiveBrain v1.2.0
Get Started
← Back to all entries
patterncsharpMinor

The cycle continues - VM's part 4

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
codereviewwpfcsharpxamlstackoverflowmvvmc#
continuescyclepartthe

Problem

So, this is my refactored VM:



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

public ObservableCollection Back { get; set; }
public ObservableCollection Forward { get; set; }
public ObservableCollection 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 _searchList = new ObservableCollection();
public ObservableCollection 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 = OneNote

Solution

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 menuItems = new List();

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

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

    public IEnumerable 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.

Code Snippets

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;
        }
    }
}
public enum AppVersion
{
    OneNoteWStore, OneNoteWPhone, Desktop2013
}
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));

        //...
    }
}
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);
    }

}

Context

StackExchange Code Review Q#80103, answer score: 4

Revisions (0)

No revisions yet.