6
\$\begingroup\$

By hashing together various online tutorials I've constructed my first MVVM application that lets users navigate between "tabs" and a "tab" (ColorList_Tab) that performs basic list functions such as adding, editing, deleting and saving colors.

Application running

A new tab can be addded by adding a new data context to the MainWindow resources and then adding the viewmodel to PageViewModels list in the MainWindowViewModel. This tab will then be represented by its Icon property from inheriting from IPageViewModel.

ColorList_Tab supports adding, editing, deleting and saving colors to a text file in a Json format. Two textboxes are used to input the color name and hex, add and edit mode can be switched between by clicking on the AddSwitchCommand and EditSwitchCommand buttons. The execute command then executes the selected mode. Edit and delete will perform the function upon the currently selected item in the Color List, with every change being saved to a text file in Json form. SampleColorCommand sets the InputHexString to the color of cursor on screen.

Besides general critique and corrections, I was hoping a few of my questions could be addressed:

  1. Is MainWindowViewModel a good way of implementing navigation in MVVM?

  2. Currently my App.Xaml.Cs is unused however I've seen examples where it is used to bind the viewmodel, which should I use?

  3. Have I correctly placed the AddColorItem, EditColorItem and DeleteColorItem methods in the Model?

  4. Should ColorClass use try and catch to create the brush or should an if else statement be used with a regex check on the hex code?

MainWindow:

<Window x:Class="MVVM_Color_Utilities.MainWindow"
        xmlns="http://schemas.microsoft.com/winfx/2006/xaml/presentation"
        xmlns:x="http://schemas.microsoft.com/winfx/2006/xaml"
        xmlns:d="http://schemas.microsoft.com/expression/blend/2008"
        xmlns:mc="http://schemas.openxmlformats.org/markup-compatibility/2006"
        xmlns:materialDesign="http://materialdesigninxaml.net/winfx/xaml/themes"
        xmlns:local="clr-namespace:MVVM_Color_Utilities"
        mc:Ignorable="d"

        xmlns:ViewModel ="clr-namespace:MVVM_Color_Utilities.ViewModel"
        xmlns:ColorList="clr-namespace:MVVM_Color_Utilities.ColorsList_Tab"

         Title="MainWindow" Height="600" Width="1000"
        WindowStartupLocation="CenterScreen" Background="LightGray"
        WindowStyle="None" AllowsTransparency="True">

    <Window.DataContext>
        <ViewModel:MainWindowViewModel/>
    </Window.DataContext>

    <Window.Resources>
        <DataTemplate DataType="{x:Type ColorList:ColorListViewModel}">
            <ColorList:ColorListView/>
        </DataTemplate>
    </Window.Resources>

    <Grid>
        <Grid.ColumnDefinitions>
            <ColumnDefinition Width="70"/>
            <ColumnDefinition Width="*"/>
        </Grid.ColumnDefinitions>
        <Grid.RowDefinitions>
            <RowDefinition Height="70"/>
            <RowDefinition Height="*"/>
        </Grid.RowDefinitions>

        <Rectangle Grid.Column="1" Fill="{StaticResource BurgundyBrush}" />
        <Rectangle Grid.RowSpan="2" Fill="{StaticResource CyanTerracottaBrush}" Margin="0,0,-0.5,0"/>

        <ItemsControl Grid.Row="1" ItemsSource="{Binding PageViewModels}" HorizontalAlignment="Stretch">
            <ItemsControl.ItemTemplate>
                <DataTemplate>
                    <Button Height="70"
                                Command="{Binding DataContext.ChangePageCommand, RelativeSource={RelativeSource AncestorType={x:Type Window}}}"
                                CommandParameter="{Binding }">
                        <materialDesign:PackIcon Kind="{Binding Icon}" Height="30" Width="30" />
                    </Button>
                </DataTemplate>
            </ItemsControl.ItemTemplate>
        </ItemsControl>

        <Rectangle  Grid.Column="1" Fill="{StaticResource BurgundyBrush}" Name="DragWindow" MouseDown="DragWindow_MouseDown"/>
        <ContentControl Grid.Column="1" Grid.RowSpan="2" Content="{Binding CurrentPageViewModel}" />

        <!--#region Window Controls-->
        <StackPanel Grid.Column="1" Orientation="Horizontal" HorizontalAlignment="Right" VerticalAlignment="Top" Height="30" x:Name="Stack" Margin="7,0,0,0">
            <!--Minimise-->
            <Button Click="MinimizeWindowButton_Click" x:Name="MinimizeWindowButton">
                <materialDesign:PackIcon Kind="Remove"  Background="{x:Null}" BorderBrush="{x:Null}"/>
            </Button>
            <!--Toggle Window State-->
            <Button Click="ChangeWindowState_Click" x:Name="ChangeWindowState" >
                <materialDesign:PackIcon Kind="CropSquare"  Background="{x:Null}" BorderBrush="{x:Null}"/>
            </Button>
            <!--Close Window-->
            <Button Click="CloseWindow_Click" Height="Auto" x:Name="CloseWindow" >
                <materialDesign:PackIcon Kind="Close"  Background="{x:Null}" BorderBrush="{x:Null}"/>
            </Button>
        </StackPanel>
<!--#endregion-->
    </Grid>
</Window>

MainWindowViewModel:

using System.Collections.Generic;
using System.Linq;
using System.Windows.Input;
using MVVM_Color_Utilities.ViewModel.Helper_Classes;

namespace MVVM_Color_Utilities.ViewModel
{
    public class MainWindowViewModel : ObservableObject
    {
        #region Fields
        private ICommand _changePageCommand;
        private IPageViewModel _currentPageViewModel;
        private List<IPageViewModel> _pageViewModels;
        #endregion

        #region Constructors
        public MainWindowViewModel()
        {
            PageViewModels.Add(new ColorsList_Tab.ColorListViewModel());
            //PageViewModels.Add(new ImageQuantizer_Tab.ImageQuantizerViewModel());

            CurrentPageViewModel = PageViewModels[0];
        }
        #endregion
        public ICommand ChangePageCommand
        {
            get
            {
                if (_changePageCommand == null)
                {
                    _changePageCommand = new RelayCommand(
                        p => ChangeViewModel((IPageViewModel)p),
                        p => p is IPageViewModel);
                }
                return _changePageCommand;
            }
        }
        public List<IPageViewModel> PageViewModels
        {
            get
            {
                if (_pageViewModels == null)
                    _pageViewModels = new List<IPageViewModel>();
                return _pageViewModels;
            }
        }
        public IPageViewModel CurrentPageViewModel
        {
            get
            {
                return _currentPageViewModel;
            }
            set
            {
                if (_currentPageViewModel != value)
                {
                    _currentPageViewModel = value;
                    OnPropertyChanged("CurrentPageViewModel");
                }
            }
        }
        private void ChangeViewModel(IPageViewModel viewModel)
        {
            if (!PageViewModels.Contains(viewModel))
            {
                PageViewModels.Add(viewModel);
            }

            CurrentPageViewModel = PageViewModels
                .FirstOrDefault(vm => vm == viewModel);
        }
    }
}

IPageViewModel Interface (Sets the icon for each viewmodel for display in the main window):

using MaterialDesignThemes.Wpf;

namespace MVVM_Color_Utilities.ViewModel.Helper_Classes
{
    public interface IPageViewModel
    {
        PackIconKind Icon { get; }
    }
}

ColorList_Tab View:

<UserControl x:Class="MVVM_Color_Utilities.ColorsList_Tab.ColorListView"
             xmlns="http://schemas.microsoft.com/winfx/2006/xaml/presentation"
             xmlns:x="http://schemas.microsoft.com/winfx/2006/xaml"
             xmlns:mc="http://schemas.openxmlformats.org/markup-compatibility/2006" 
             xmlns:d="http://schemas.microsoft.com/expression/blend/2008" 
             xmlns:local="clr-namespace:MVVM_Color_Utilities.ColorsList_Tab"
             xmlns:materialDesign="http://materialdesigninxaml.net/winfx/xaml/themes"
             mc:Ignorable="d" 
             >
    <UserControl.InputBindings>
        <KeyBinding Key="Enter" Command="{Binding ExecuteCommand}"/>
        <KeyBinding Modifiers="Ctrl" Key="D" Command="{Binding DeleteItem}"/>
        <KeyBinding Modifiers="Ctrl" Key="A" Command="{Binding AddSwitchCommand}"/>
        <KeyBinding Modifiers="Ctrl" Key="E" Command="{Binding EditSwitchCommand}"/>
        <KeyBinding Modifiers="Ctrl" Key="Q" Command="{Binding SampleColorCommand}"/>
    </UserControl.InputBindings>

    <Grid>
        <Grid.RowDefinitions>
            <RowDefinition Height="70"/>
            <RowDefinition Height="50"/>
            <RowDefinition Height="*"/>
        </Grid.RowDefinitions>

        <Grid Width="270" HorizontalAlignment="Left">
            <Grid.ColumnDefinitions>
                <ColumnDefinition Width="*"/>
                <ColumnDefinition Width="*"/>
                <ColumnDefinition Width="*"/>
            </Grid.ColumnDefinitions>

<!--This changes the "background" of the buttons as setting the background of buttons 
            while using material design causes an ugly shadow effect-->
            <Rectangle Grid.Column="0" >
                <Rectangle.Style>
                    <Style TargetType="Rectangle">
                        <Setter Property="Fill" Value="{StaticResource BurgundyBrush}"/>
                        <Style.Triggers>
                            <DataTrigger Binding="{Binding AddingModeBool}" Value="True">
                                <Setter Property="Fill" Value="{StaticResource BurgundyLightBrush}"/>
                            </DataTrigger>
                        </Style.Triggers>
                    </Style>
                </Rectangle.Style>
            </Rectangle>

            <Rectangle Grid.Column="1">
                <Rectangle.Style>
                    <Style TargetType="Rectangle">
                        <Setter Property="Fill" Value="{StaticResource BurgundyLightBrush}"/>
                        <Style.Triggers>
                            <DataTrigger Binding="{Binding AddingModeBool}" Value="True">
                                <Setter Property="Fill" Value="{StaticResource BurgundyBrush}"/>
                            </DataTrigger>
                        </Style.Triggers>
                    </Style>
                </Rectangle.Style>
            </Rectangle>

            <Button Grid.Column="0" Command="{Binding Path=AddSwitchCommand}">
                <materialDesign:PackIcon Kind="Add"/>
            </Button>
            <Button Grid.Column="1" Command="{Binding Path=EditSwitchCommand}"  >
                <materialDesign:PackIcon Kind="Edit" />
            </Button>
            <Button Grid.Column="2" Command="{Binding Path=DeleteItem}">
                <materialDesign:PackIcon Kind="Delete" VerticalAlignment="Center"/>
            </Button>
        </Grid>

<!--#region ListBox Header-->
        <Grid Grid.Row="1" Background="{StaticResource BurgundyLightBrush}" >
            <Grid.ColumnDefinitions>
                <ColumnDefinition Width="0.8*"/>
                <ColumnDefinition Width="1.5*"/>
                <ColumnDefinition Width="*"/>
                <ColumnDefinition Width="*"/>
                <ColumnDefinition Width="0.7*"/>
                <ColumnDefinition Width="1.2*"/>
            </Grid.ColumnDefinitions>
            <Grid.Resources>
                <Style TargetType="TextBox">
                    <Setter Property="Background" Value="{StaticResource BurgundyFaintBrush}"/>
                    <Setter Property="Height" Value="20"/>
                </Style>
            </Grid.Resources>

            <Rectangle Height="18" Width="60" Fill="White"/>
            <Rectangle Height="18" Width="60" Fill="{Binding IndicatorBrush}" Stroke="Gray" StrokeThickness="0.4"/>

            <TextBox Grid.Column="1" Width="160" Text="{Binding InputName, UpdateSourceTrigger=PropertyChanged}" />
            <TextBox Grid.Column="2" Width="80" Text="{Binding InputHex, UpdateSourceTrigger=PropertyChanged}"/>

            <Button Grid.Column="4" Command="{Binding Path=SampleColorCommand}">
                <materialDesign:PackIcon Kind="Colorize" Width="40"/>
            </Button>
            <Button Grid.Column="5" Command="{Binding Path=ExecuteCommand}">
                <materialDesign:PackIcon Kind="done" Width="100"/>
            </Button>
        </Grid>
        <!--#endregion-->

<!--#region ListBox-->
        <ListBox Grid.Row="2" ItemsSource="{Binding ColorListSource}" SelectedIndex="{Binding SelectedItemIndex}">
            <ListBox.ItemContainerStyle>
                <Style TargetType="ListBoxItem" BasedOn="{StaticResource {x:Type ListBoxItem}}">
                    <Setter Property="HorizontalContentAlignment" Value="Stretch"/>
                    <Style.Triggers>
                        <MultiTrigger>
                            <MultiTrigger.Conditions>
                                <Condition Property="IsSelected" Value="true"/>
                                <Condition Property="Selector.IsSelectionActive" Value="true"/>
                            </MultiTrigger.Conditions>
                            <Setter Property="Background" Value="Gray"/>
                        </MultiTrigger>
                    </Style.Triggers>
                </Style>
            </ListBox.ItemContainerStyle>
            <ListBox.ItemTemplate>
                <DataTemplate>
                    <Grid
                        >
                        <Grid.ColumnDefinitions>
                            <ColumnDefinition Width="0.8*"/>
                            <ColumnDefinition Width="1.5*"/>
                            <ColumnDefinition Width="*"/>
                            <ColumnDefinition Width="*"/>
                            <ColumnDefinition Width="0.7*"/>
                            <ColumnDefinition Width="1.2*"/>
                        </Grid.ColumnDefinitions>
                        <Rectangle  Height="18" Width="60" Fill="{Binding SampleBrush}" Stroke="Gray" StrokeThickness="0.4"/>
                        <TextBlock Text="{Binding Name}" Width="160" Grid.Column="1"/>
                        <TextBlock Text="{Binding Hex}" Width="70" Grid.Column="2"/>
                    </Grid>
                </DataTemplate>
            </ListBox.ItemTemplate>
        </ListBox>
<!--#endregion-->
    </Grid>
</UserControl>

ColorList_Tab ViewModel:

using System.Collections.ObjectModel;   
using MVVM_Color_Utilities.ViewModel.Helper_Classes;
using MaterialDesignThemes.Wpf;
using System.Windows.Media;
using System.Windows.Input;
using MVVM_Color_Utilities.Helpers;
using System.Text.RegularExpressions;

namespace MVVM_Color_Utilities.ColorsList_Tab
{
    class ColorListViewModel : ObservableObject, IPageViewModel
    {
        private readonly ColorListModel model = new ColorListModel();
        private Regex _hexReg = new Regex("^#([0-9a-fA-F]{0,8})?$"); //"^#(?:(?:[0-9a-fA-F]{3}){1,2}|(?:[0-9a-fA-F]{4}){1,2})$"

        private bool _addingModeBool = true;
        private int _selectedItemIndex;

        private string _inputNameString;
        private string _inputHexString;

        private SolidColorBrush _inputBrush = Brushes.
        private ICommand _addSwitchCommand;
        private ICommand _editSwitchCommand;

        private ICommand _executeCommand;
        private ICommand _sampleColorCommand;
        private ICommand _deleteItemCommand;

        public ColorListViewModel()
        {
            SelectedItemIndex = 0;
        }
        public SolidColorBrush IndicatorBrush
        {
            get
            {
                return _inputBrush;
            }
            set
            {
                _inputBrush = value;
                OnPropertyChanged("IndicatorBrush");
            }
        }
        public string InputName
        {
            get
            {
                return _inputNameString;
            }
            set
            {
                _inputNameString = value;
                OnPropertyChanged("InputName");
            }
        }
        public string InputHex
        {
            get
            {
                return _inputHexString;
            }
            set
            {
                if (_hexReg.IsMatch(value)||value=="")//Only allows valid hex charcters ie start with # and the 1-9a-f
                {
                    _inputHexString = value;
                    OnPropertyChanged("InputHex");
                }
                try
                {
                    //Sets indicator to the new color
                    IndicatorBrush = new SolidColorBrush((Color)ColorConverter.ConvertFromString(value));
                }
                catch { }
            }
        }
        public PackIconKind Icon => PackIconKind.Palette;
        public bool AddingModeBool
        {
            get
            {
                return _addingModeBool;
            }
            set
            {
                _addingModeBool = value;
                OnPropertyChanged("AddingModeBool");
            }
        }
        public ObservableCollection<ColorClass> ColorListSource
        {
            get
            {
                return model.ColorClassList;
            }
        }
        public int SelectedItemIndex
        {
            get
            {
                if (_selectedItemIndex >= ColorListSource.Count && ColorListSource.Count != 0)
                {
                    _selectedItemIndex = ColorListSource.Count - 1;
                }
                return _selectedItemIndex;
            }
            set
            {
                _selectedItemIndex= MathUtils.Clamp(0, ColorListSource.Count - 1, value);
                if (ColorListSource.Count > 0)
                {
                    InputHex = ColorListSource[_selectedItemIndex].Hex;
                    InputName = ColorListSource[_selectedItemIndex].Name;
                }
                else
                {
                    InputHex = "";
                    InputName = "";
                }
                OnPropertyChanged("SelectedItemIndex");
            }
        }
        public ICommand AddSwitchCommand
        {
            get
            {
                if (_addSwitchCommand == null)
                {
                    _addSwitchCommand = new RelayCommand(param => AddSwitchMethod());
                }
                return _addSwitchCommand;
            }
        }
        public ICommand EditSwitchCommand
        {
            get
            {
                if(_editSwitchCommand == null)
                {
                    _editSwitchCommand = new RelayCommand(param => EditSwitchMethod());
                }
                return _editSwitchCommand;
            }
        }
        public ICommand ExecuteCommand
        {
            get
            {
                if (_executeCommand == null)
                {
                    _executeCommand = new RelayCommand(param => ExecuteMethod());
                }
                return _executeCommand;
            }
        }
        public ICommand SampleColorCommand
        {
            get
            {
                if (_sampleColorCommand == null)
                {
                    _sampleColorCommand = new RelayCommand(param => SampleColorMethod());
                }
                return _sampleColorCommand;
            }

        }
        public ICommand DeleteItem
        {
            get
            {
                if (_deleteItemCommand == null)
                {
                    _deleteItemCommand = new RelayCommand(param => DeleteItemMethod());
                }
                return _deleteItemCommand;
            }
        }
        void AddSwitchMethod()
        {
            AddingModeBool = true;
        }
        void EditSwitchMethod()
        {
            AddingModeBool = false;
        }
        void ExecuteMethod()
        {
            if (AddingModeBool)
                AddNewItemMethod();
            else
                EditItemMethod();
        }
        void AddNewItemMethod()
        {
            model.AddColorItem(SelectedItemIndex, InputHex, InputName);
            SelectedItemIndex = 0;
        }
        void EditItemMethod()
        {
            model.EditColorItem(SelectedItemIndex, InputHex, InputName);
        }
        void DeleteItemMethod()
        { 
            model.DeleteColorItem(SelectedItemIndex);
        }
        void SampleColorMethod()
        {
            Color color =  ColorUtils.GetCursorColor();
            InputHex = $"#{color.R:X2}{color.G:X2}{color.B:X2}";
        }
    }
}

Observable Objects (not my code)

using System.ComponentModel;

namespace MVVM_Color_Utilities.ViewModel.Helper_Classes
{
    public abstract class ObservableObject : INotifyPropertyChanged
    {
        public event PropertyChangedEventHandler PropertyChanged;

        protected void OnPropertyChanged(string propertyName)
        {
            var handler = PropertyChanged;
            if (PropertyChanged != null)
            {
                handler(this, new PropertyChangedEventArgs(propertyName));
            }
        }
    }
}

RelayCommand (not my code) :

using System;
using System.Diagnostics;
using System.Windows.Input;

namespace MVVM_Color_Utilities.ViewModel.Helper_Classes
{
    public class RelayCommand : ICommand
    {
        #region Fields
        readonly Action<object> _execute;
        readonly Predicate<object> _canExecute;
        #endregion // Fields

        #region Constructors
        public RelayCommand(Action<object> execute)
            : this(execute, null)
        {
        }
        public RelayCommand(Action<object> execute, Predicate<object> canExecute)
        {
            if (execute == null)
            {
                throw new ArgumentNullException("execute");
            }
            _execute = execute;
            _canExecute = canExecute;
        }

        #endregion // Constructors

        #region ICommand Members

        [DebuggerStepThrough]
        public bool CanExecute(object parameters)
        {
            return _canExecute == null ? true : _canExecute(parameters);
        }

        public event EventHandler CanExecuteChanged
        {
            add { CommandManager.RequerySuggested += value; }
            remove { CommandManager.RequerySuggested -= value; }
        }

        public void Execute(object parameters)
        {
            _execute(parameters);
        }

        #endregion // ICommand Members
    }
}

ColorList_Tab Model:

using System.IO;
using System.Collections.ObjectModel;
using System.Windows.Media;
using Newtonsoft.Json;

namespace MVVM_Color_Utilities.ColorsList_Tab
{
    public class ColorListModel
    {
        #region Fields
        private readonly static string projectPath = Directory.GetParent(Directory.GetCurrentDirectory()).Parent.FullName; //Get Path of ColorItems file
        private readonly static string colorsFilePath = projectPath + "/Resources/ColorItemsList.txt";
        #endregion

        #region Properties
        public ObservableCollection<ColorClass> ColorClassList { get; }
        =JsonConvert.DeserializeObject<ObservableCollection<ColorClass>>(File.ReadAllText(colorsFilePath));

        public int NextID
        {
            get
            {
                return ColorClassList.Count > 0 ? ColorClassList[0].ID + 1 : 0;
            }
        }
        #endregion

        #region Methods
        private void SaveColorsList()
        {
            try
            {
                File.WriteAllText(colorsFilePath, JsonConvert.SerializeObject(ColorClassList));
            }
            catch { }
        }
        public void AddColorItem(int index,string hexString, string nameString)
        {
            if(ColorClassList.Count > index || ColorClassList.Count==0)
            {
                ColorClassList.Insert(0, new ColorClass(NextID, hexString, nameString));
                SaveColorsList();
            }
        }
        public void EditColorItem(int index,string hexString, string nameString)
        {
            if (ColorClassList.Count > index && ColorClassList.Count > 0)
            {
                ColorClassList[index] = new ColorClass(NextID, hexString, nameString);
                SaveColorsList();
            }
        }
        public void DeleteColorItem(int index)
        {
            if (ColorClassList.Count > index &&ColorClassList.Count>0)
            {
                ColorClassList.RemoveAt(index);
                SaveColorsList();
            }
        }
        #endregion
    }
}

ColorClass:

    public class ColorClass
    {
        public ColorClass(int id, string hex, string name)
        {
            ID = id;
            Name = name;
            Hex = hex;
        }

        public int ID { get; set; }
        public string Hex { get; set; }
        public string Name { get; set; }

        public SolidColorBrush SampleBrush
        {
            get
            {
                Color color;
                try
                {
                     color = (Color)ColorConverter.ConvertFromString(Hex);
                }
                catch//Invalid hex defaults to white.
                {
                    color = (Color)ColorConverter.ConvertFromString("#FFFF");
                    Hex = "#FFFF";
                }
                return new SolidColorBrush(color);
            } 
        }
    }
\$\endgroup\$
  • \$\begingroup\$ Do you happen to have this on GitHub too? \$\endgroup\$ – t3chb0t Sep 8 at 15:50
  • 1
    \$\begingroup\$ Yes, however it does have two additional unfinished tabs. github.com/TimothyMakkison/MVVM-Color-Utilities . Sorry for the long question. \$\endgroup\$ – DaemonFire Sep 8 at 15:58
  • 1
    \$\begingroup\$ Don't worry about the length, the question is great and as a matter of fact I wanted to fork it ;-] \$\endgroup\$ – t3chb0t Sep 8 at 16:03
  • 2
    \$\begingroup\$ Is RelayCommand your code or picked from the internet? It's ok to use third-party classes, it's just so we know whether we should review that class. \$\endgroup\$ – dfhwze Sep 8 at 16:54
  • 1
    \$\begingroup\$ I did review these 2 classes to challenge you that maybe creating your own classes would be a better idea than using these standard snippet classes. \$\endgroup\$ – dfhwze Sep 8 at 17:31
4
\$\begingroup\$

Preface

There is a lot to review. This review is focused on the parts that are not related to the View or ViewModel.

Note: You have also edited the question concurrently with me making this review to state ObservableObject and RelayCommand are third-party classes. That's fine by me, since I still wanted to point out to you you shouldn't just copy these classes from internet and use them without any changes or proper consideration.


ColorListModel

You cannot reuse this class in scenarios where the paths are different than below. Also, when changing paths, you'd have to update this code with it. Consider reading these paths from a settings or configuration file.

private readonly static string projectPath 
    = Directory.GetParent(Directory.GetCurrentDirectory())...
private readonly static string colorsFilePath 
    = projectPath + "/Resources/ColorItemsList.txt";

Consider using the lazy pattern to load properties on first demand to avoid unnecessary resources when not required: property ColorClassList.

SaveColorsList swallows all exceptions. At least log something or return a bool in sandbox mode if you don't like this method to throw exceptions.

AddColorItem, EditColorItem and DeleteColorItem only execute when the index is in bounds. The caller does not get feedback about out-of-bounds. Throw an exception or return a bool to let caller handle edge cases. Furthermore, AddColorItem does not use index as it stores on index 0 instead. Is this as designed?


(third-party code)

ObservableObject

This class is a design decision that I would challenge. It provides insufficient context to be a good candidate for a base class for other classes. I put it in the list of classes as ComparableBase, EquatableBase, DisposableBase. Think about whether a common base clase is really helpful here.

In addition, this class provides a public event PropertyChanged but never disposes it. Even if the WPF framework is able to subscribe and unsubscribe correctly from it, your own code-behind and other application logic is also allowed to subscribe. Classes that provide events should in my opinion always implement IDisposable.

RelayCommand

This is a famous allround command. I think it originates from Telerik, but several other variants are out there as well (DelegateCommand for instance). I would mention the use of third-party code in a question so we know how to review it.

This pattern with Action<object> execute is contravariant, but since object is the deepest base class, it's not that useful. For instance, you cannot exchange execute with an Action<string>. For this reason, consider creating also a RelayCommand<T> that accepts Action<T> execute. This class is more usable for code-behind and other application logic.

The predicate parameter should be made optional. You might also like to create an AsyncRelayCommand<T> with a composite CancelCommand (Example).

\$\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.