From 2d3dc307cc204d789db139c822189d5905cb92cd Mon Sep 17 00:00:00 2001 From: Benjamin Popp Date: Tue, 27 Nov 2018 21:32:19 -0600 Subject: [PATCH] Refactor EditorViewModel constructor. Pull out command implementations into helper. Single-source the list of commands that need to be refreshed on tab change. Refactor ViewPort constructor. Pull out command implemenatations into helper. Use CanAlwaysExecute instead of arg => true Move CanAlwaysExecute to command extensions. Change 'EditorNotifiesCanExecuteChangedOnTabChange' to a theory, so I can more easily add new commands to test for in the future. Bug fix: EditorViewModel should notify copy/delete commands when tab changes. --- Gen3Hex/ViewModel/EditorViewModel.cs | 206 +++++++++++++++------------ Gen3Hex/ViewModel/ViewPort.cs | 37 ++--- HavenSoft/ICommandExtensions.cs | 5 + HexTests/GeneralAppTests.cs | 25 ++-- 4 files changed, 153 insertions(+), 120 deletions(-) diff --git a/Gen3Hex/ViewModel/EditorViewModel.cs b/Gen3Hex/ViewModel/EditorViewModel.cs index be57fe09..3134cd45 100644 --- a/Gen3Hex/ViewModel/EditorViewModel.cs +++ b/Gen3Hex/ViewModel/EditorViewModel.cs @@ -5,15 +5,41 @@ using System.Collections.Generic; using System.Collections.Specialized; using System.Linq; using System.Windows.Input; +using static HavenSoft.ICommandExtensions; namespace HavenSoft.Gen3Hex.ViewModel { public class EditorViewModel : ViewModelCore, IEnumerable, INotifyCollectionChanged { private readonly IFileSystem fileSystem; private readonly List tabs; - private readonly StubCommand newCommand, open, save, saveAs, saveAll, close, closeAll; - private readonly StubCommand undo, redo, cut, copy, paste, delete; - private readonly StubCommand back, forward, gotoCommand, showGoto, find, findPrevious, findNext, showFind, clearError; + + private readonly List commandsToRefreshOnTabChange = new List(); + private readonly StubCommand + newCommand = new StubCommand(), + open = new StubCommand(), + save = new StubCommand(), + saveAs = new StubCommand(), + saveAll = new StubCommand(), + close = new StubCommand(), + closeAll = new StubCommand(), + + undo = new StubCommand(), + redo = new StubCommand(), + cut = new StubCommand(), + copy = new StubCommand(), + paste = new StubCommand(), + delete = new StubCommand(), + + back = new StubCommand(), + forward = new StubCommand(), + gotoCommand = new StubCommand(), + showGoto = new StubCommand(), + find = new StubCommand(), + findPrevious = new StubCommand(), + findNext = new StubCommand(), + showFind = new StubCommand(), + clearError = new StubCommand(); + private readonly Dictionary, EventHandler> forwardExecuteChangeNotifications; private (IViewPort tab, int)[] recentFindResults; @@ -91,6 +117,7 @@ namespace HavenSoft.Gen3Hex.ViewModel { public int Count => tabs.Count; private int selectedIndex; + public int SelectedIndex { get => selectedIndex; set { @@ -109,99 +136,24 @@ namespace HavenSoft.Gen3Hex.ViewModel { tabs = new List(); selectedIndex = -1; - bool CanAlwaysExecute(object arg) => true; + ImplementCommands(); - newCommand = new StubCommand { - CanExecute = CanAlwaysExecute, - Execute = arg => Add(new ViewPort()), - }; - open = new StubCommand { - CanExecute = CanAlwaysExecute, - Execute = arg => { - var file = arg as LoadedFile ?? fileSystem.OpenFile(); - if (file == null) return; - Add(new ViewPort(file)); - }, - }; - gotoCommand = new StubCommand { - CanExecute = arg => SelectedTab?.Goto?.CanExecute(arg) ?? false, - Execute = arg => { - SelectedTab?.Goto?.Execute(arg); - GotoControlVisible = false; - }, - }; - showGoto = new StubCommand { - CanExecute = CanAlwaysExecute, - Execute = arg => GotoControlVisible = (bool)arg, - }; - find = new StubCommand { - CanExecute = CanAlwaysExecute, - Execute = arg => FindExecuted((string)arg), - }; - findPrevious = new StubCommand { - CanExecute = arg => recentFindResults?.Length != 0, - Execute = arg => { - int attemptCount = 0; - while (attemptCount < recentFindResults.Length) { - attemptCount++; - currentFindResultIndex--; - if (currentFindResultIndex < 0) currentFindResultIndex += recentFindResults.Length; - var (tab, offset) = recentFindResults[currentFindResultIndex]; - if (tab != SelectedTab) continue; - tab.Goto.Execute(offset.ToString("X2")); - break; - } - }, - }; - findNext = new StubCommand { - CanExecute = arg => recentFindResults?.Length != 0, - Execute = arg => { - int attemptCount = 0; - while (attemptCount < recentFindResults.Length) { - attemptCount++; - currentFindResultIndex++; - if (currentFindResultIndex >= recentFindResults.Length) currentFindResultIndex -= recentFindResults.Length; - var (tab, offset) = recentFindResults[currentFindResultIndex]; - if (tab != SelectedTab) continue; - tab.Goto.Execute(offset.ToString("X2")); - break; - } - }, - }; - showFind = new StubCommand { - CanExecute = CanAlwaysExecute, - Execute = arg => FindControlVisible = (bool)arg, - }; - clearError = new StubCommand { - CanExecute = arg => showError, - Execute = arg => ErrorMessage = string.Empty, - }; - cut = new StubCommand { - CanExecute = arg => SelectedTab?.Copy?.CanExecute(arg) ?? false, - Execute = arg => { - if (SelectedTab != null && SelectedTab.Copy != null && SelectedTab.Clear != null) { - SelectedTab.Copy.Execute(fileSystem); - SelectedTab.Clear.Execute(); - } - } - }; copy = CreateWrapperForSelected(tab => tab.Copy); - paste = new StubCommand { - CanExecute = arg => SelectedTab is ViewPort, - Execute = arg => (SelectedTab as ViewPort)?.Edit(fileSystem.CopyText), - }; delete = CreateWrapperForSelected(tab => tab.Clear); save = CreateWrapperForSelected(tab => tab.Save); saveAs = CreateWrapperForSelected(tab => tab.SaveAs); - saveAll = CreateWrapperForAll(tab => tab.Save); close = CreateWrapperForSelected(tab => tab.Close); - closeAll = CreateWrapperForAll(tab => tab.Close); undo = CreateWrapperForSelected(tab => tab.Undo); redo = CreateWrapperForSelected(tab => tab.Redo); back = CreateWrapperForSelected(tab => tab.Back); forward = CreateWrapperForSelected(tab => tab.Forward); + saveAll = CreateWrapperForAll(tab => tab.Save); + closeAll = CreateWrapperForAll(tab => tab.Close); + forwardExecuteChangeNotifications = new Dictionary, EventHandler> { + { tab => tab.Copy, (sender, e) => copy.CanExecuteChanged.Invoke(this, e) }, + { tab => tab.Clear, (sender, e) => delete.CanExecuteChanged.Invoke(this, e) }, { tab => tab.Save, (sender, e) => save.CanExecuteChanged.Invoke(this, e) }, { tab => tab.SaveAs, (sender, e) => saveAs.CanExecuteChanged.Invoke(this, e) }, { tab => tab.Close, (sender, e) => close.CanExecuteChanged.Invoke(this, e) }, @@ -212,6 +164,79 @@ namespace HavenSoft.Gen3Hex.ViewModel { }; } + private void ImplementCommands() { + newCommand.CanExecute = CanAlwaysExecute; + newCommand.Execute = arg => Add(new ViewPort()); + + open.CanExecute = CanAlwaysExecute; + open.Execute = arg => { + var file = arg as LoadedFile ?? fileSystem.OpenFile(); + if (file == null) return; + Add(new ViewPort(file)); + }; + + gotoCommand.CanExecute = arg => SelectedTab?.Goto?.CanExecute(arg) ?? false; + gotoCommand.Execute = arg => { + SelectedTab?.Goto?.Execute(arg); + GotoControlVisible = false; + }; + + showGoto.CanExecute = CanAlwaysExecute; + showGoto.Execute = arg => GotoControlVisible = (bool)arg; + + ImplementFindCommands(); + + clearError.CanExecute = arg => showError; + clearError.Execute = arg => ErrorMessage = string.Empty; + + cut.CanExecute = arg => SelectedTab?.Copy?.CanExecute(arg) ?? false; + cut.Execute = arg => { + if (SelectedTab != null && SelectedTab.Copy != null && SelectedTab.Clear != null) { + SelectedTab.Copy.Execute(fileSystem); + SelectedTab.Clear.Execute(); + } + }; + + paste.CanExecute = arg => SelectedTab is ViewPort; + paste.Execute = arg => (SelectedTab as ViewPort)?.Edit(fileSystem.CopyText); + } + + private void ImplementFindCommands() { + find.CanExecute = CanAlwaysExecute; + find.Execute = arg => FindExecuted((string)arg); + + findPrevious.CanExecute = arg => recentFindResults?.Length != 0; + findPrevious.Execute = arg => { + int attemptCount = 0; + while (attemptCount < recentFindResults.Length) { + attemptCount++; + currentFindResultIndex--; + if (currentFindResultIndex < 0) currentFindResultIndex += recentFindResults.Length; + var (tab, offset) = recentFindResults[currentFindResultIndex]; + if (tab != SelectedTab) continue; + tab.Goto.Execute(offset.ToString("X2")); + break; + } + }; + + findNext.CanExecute = arg => recentFindResults?.Length != 0; + findNext.Execute = arg => { + int attemptCount = 0; + while (attemptCount < recentFindResults.Length) { + attemptCount++; + currentFindResultIndex++; + if (currentFindResultIndex >= recentFindResults.Length) currentFindResultIndex -= recentFindResults.Length; + var (tab, offset) = recentFindResults[currentFindResultIndex]; + if (tab != SelectedTab) continue; + tab.Goto.Execute(offset.ToString("X2")); + break; + } + }; + + showFind.CanExecute = CanAlwaysExecute; + showFind.Execute = arg => FindControlVisible = (bool)arg; + } + public void Add(ITabContent content) { tabs.Add(content); SelectedIndex = tabs.Count - 1; @@ -256,6 +281,8 @@ namespace HavenSoft.Gen3Hex.ViewModel { } }; + commandsToRefreshOnTabChange.Add(command); + return command; } @@ -344,16 +371,7 @@ namespace HavenSoft.Gen3Hex.ViewModel { } private void StartListeningToCommandsFromCurrentTab() { - var commandsToRefresh = new List { - undo, - redo, - save, - saveAs, - close, - back, - forward, - }; - commandsToRefresh.ForEach(command => command.CanExecuteChanged.Invoke(command, EventArgs.Empty)); + commandsToRefreshOnTabChange.ForEach(command => command.CanExecuteChanged.Invoke(command, EventArgs.Empty)); if (selectedIndex == -1) return; diff --git a/Gen3Hex/ViewModel/ViewPort.cs b/Gen3Hex/ViewModel/ViewPort.cs index dac43922..993a4f85 100644 --- a/Gen3Hex/ViewModel/ViewPort.cs +++ b/Gen3Hex/ViewModel/ViewPort.cs @@ -8,8 +8,8 @@ using System.ComponentModel; using System.Globalization; using System.IO; using System.Linq; -using System.Text; using System.Windows.Input; +using static HavenSoft.ICommandExtensions; namespace HavenSoft.Gen3Hex.ViewModel { /// @@ -146,7 +146,10 @@ namespace HavenSoft.Gen3Hex.ViewModel { #region Saving - private readonly StubCommand save, saveAs, close; + private readonly StubCommand + save = new StubCommand(), + saveAs = new StubCommand(), + close = new StubCommand(); public ICommand Save => save; @@ -220,7 +223,12 @@ namespace HavenSoft.Gen3Hex.ViewModel { history = new ChangeHistory>(RevertChanges); history.PropertyChanged += HistoryPropertyChanged; - clear.CanExecute = arg => true; + ImplementCommands(); + RefreshBackingData(); + } + + private void ImplementCommands() { + clear.CanExecute = CanAlwaysExecute; clear.Execute = arg => { var selectionStart = scroll.ViewPointToDataIndex(selection.SelectionStart); var selectionEnd = scroll.ViewPointToDataIndex(selection.SelectionEnd); @@ -230,7 +238,7 @@ namespace HavenSoft.Gen3Hex.ViewModel { RefreshBackingData(); }; - copy.CanExecute = arg => true; + copy.CanExecute = CanAlwaysExecute; copy.Execute = arg => { var selectionStart = scroll.ViewPointToDataIndex(selection.SelectionStart); var selectionEnd = scroll.ViewPointToDataIndex(selection.SelectionEnd); @@ -239,20 +247,15 @@ namespace HavenSoft.Gen3Hex.ViewModel { var bytes = Enumerable.Range(left, length).Select(i => data[i]); ((IFileSystem)arg).CopyText = string.Join(" ", bytes.Select(value => value.ToString("X2"))); }; - save = new StubCommand { - CanExecute = arg => !history.IsSaved, - Execute = arg => SaveExecuted((IFileSystem)arg), - }; - saveAs = new StubCommand { - CanExecute = arg => true, - Execute = arg => SaveAsExecuted((IFileSystem)arg), - }; - close = new StubCommand { - CanExecute = arg => true, - Execute = arg => CloseExecuted((IFileSystem)arg), - }; - RefreshBackingData(); + save.CanExecute = arg => !history.IsSaved; + save.Execute = arg => SaveExecuted((IFileSystem)arg); + + saveAs.CanExecute = CanAlwaysExecute; + saveAs.Execute = arg => SaveAsExecuted((IFileSystem)arg); + + close.CanExecute = CanAlwaysExecute; + close.Execute = arg => CloseExecuted((IFileSystem)arg); } public bool IsSelected(Point point) => selection.IsSelected(point); diff --git a/HavenSoft/ICommandExtensions.cs b/HavenSoft/ICommandExtensions.cs index 8660a03a..7fe1a636 100644 --- a/HavenSoft/ICommandExtensions.cs +++ b/HavenSoft/ICommandExtensions.cs @@ -6,5 +6,10 @@ namespace HavenSoft { /// Runs execute on the command with a null parameter. /// public static void Execute(this ICommand command) => command.Execute(null); + + /// + /// Utility implementation of CanExecute for commands that can always execute. + /// + public static bool CanAlwaysExecute(object sender) => true; } } diff --git a/HexTests/GeneralAppTests.cs b/HexTests/GeneralAppTests.cs index 7498727c..859c8f2b 100644 --- a/HexTests/GeneralAppTests.cs +++ b/HexTests/GeneralAppTests.cs @@ -204,23 +204,30 @@ namespace HavenSoft.HexTests { Assert.Equal(7, count); } - [Fact] - public void EditorNotifiesCanExecuteChangedOnTabChange() { + + [Theory] + [InlineData("Copy")] + [InlineData("Delete")] + [InlineData("Save")] + [InlineData("SaveAs")] + [InlineData("Close")] + [InlineData("Undo")] + [InlineData("Redo")] + [InlineData("Back")] + [InlineData("Forward")] + public void EditorNotifiesCanExecuteChangedOnTabChange(string commandName) { int count = 0; - editor.Save.CanExecuteChanged += (sender, e) => count++; - editor.SaveAs.CanExecuteChanged += (sender, e) => count++; - editor.Close.CanExecuteChanged += (sender, e) => count++; - editor.Undo.CanExecuteChanged += (sender, e) => count++; - editor.Redo.CanExecuteChanged += (sender, e) => count++; + var command = (ICommand)editor.GetType().GetProperty(commandName).GetValue(editor); + command.CanExecuteChanged += (sender, e) => count++; var tab = new StubTabContent(); tab.Close = new StubCommand { CanExecute = arg => true, Execute = arg => tab.Closed.Invoke(tab, EventArgs.Empty) }; editor.Add(tab); - Assert.Equal(5, count); + Assert.Equal(1, count); count = 0; editor.Close.Execute(); - Assert.Equal(5, count); + Assert.Equal(1, count); } [Fact]