Round 3 of unit test conversions and modernization work

src/MenuTracker.cs

@@ -47,6 +47,22 @@ public void Register(MenuBar mb)
         this.bars.Add(mb);
     }
 
+    /// <summary>
+    /// Unregisters listeners for <paramref name="mb"/>.
+    /// </summary>
+    /// <param name="mb"><see cref="MenuBar"/> to stop tracking.</param>
+    public void UnregisterMenuBar( MenuBar? mb )
+    {
+        if ( !bars.TryTake( out mb ) )
+        {
+            return;
+        }
+
+        mb.MenuAllClosed -= MenuAllClosed;
+        mb.MenuOpened -= MenuOpened;
+        mb.MenuClosing -= MenuClosing;
+    }
+
     /// <summary>
     /// <para>
     /// Searches child items of all MenuBars tracked by this class
@@ -63,7 +79,7 @@ public void Register(MenuBar mb)
     /// <returns>The immediate parent of <paramref name="item"/>.</returns>
     /// <remarks>Result may be a top level menu (e.g. File, View)
     /// or a sub-menu parent (e.g. View=>Windows).</remarks>
-    public MenuBarItem? GetParent( MenuItem item, out MenuBar? hostBar )
+    private MenuBarItem? GetParent( MenuItem item, out MenuBar? hostBar )
     {
         foreach (var bar in this.bars)
         {
@@ -83,7 +99,23 @@ public void Register(MenuBar mb)
         return null;
     }
 
-    public bool TryGetParent(MenuItem item, [NotNullWhen(true)]out MenuBar? hostBar, [NotNullWhen(true)] out MenuBarItem? parentItem)
+    /// <summary>
+    ///   Searches child items of all MenuBars tracked by this class to try and find the parent of the item passed.
+    /// </summary>
+    /// <param name="item">The item whose parent you want to find.</param>
+    /// <param name="hostBar">
+    ///   When this method returns true, the <see cref="MenuBar" /> that owns <paramref name="item" />.<br /> Otherwise, <see langword="null" /> if
+    ///   not found or parent not registered (see <see cref="Register(MenuBar)" />).
+    /// </param>
+    /// <param name="parentItem">
+    ///   When this method returns <see langword="true" />, the immediate parent of <paramref name="item" />.<br /> Otherwise,
+    ///   <see langword="null" />
+    /// </param>
+    /// <remarks>
+    ///   Search is recursive and dips into sub-menus.<br /> For sub-menus it is the immediate parent that is returned.
+    /// </remarks>
+    /// <returns>A <see langword="bool" /> indicating if the search was successful or not.</returns>
+    public bool TryGetParent( MenuItem item, [NotNullWhen( true )] out MenuBar? hostBar, [NotNullWhen( true )] out MenuBarItem? parentItem )
     {
         var parentCandidate = GetParent( item, out hostBar );
         if ( parentCandidate is null )
@@ -106,22 +138,21 @@ public bool TryGetParent(MenuItem item, [NotNullWhen(true)]out MenuBar? hostBar,
     /// the substitution object (<see cref="MenuItem"/>).  See
     /// <see cref="ConvertMenuBarItemToRegularItemIfEmpty(MenuBarItem, out MenuItem?)"/>
     /// for more information.</returns>
-    public Dictionary<MenuBarItem, MenuItem> ConvertEmptyMenus()
+    public Dictionary<MenuBarItem, MenuItem> ConvertEmptyMenus( )
     {
-        var toReturn = new Dictionary<MenuBarItem, MenuItem>();
-
+        Dictionary<MenuBarItem, MenuItem> dictionary = [];
         foreach (var b in this.bars)
         {
             foreach (var bi in b.Menus)
             {
-                foreach (var converted in this.ConvertEmptyMenus(b, bi))
+                foreach ( ( MenuBarItem? convertedMenuBarItem, MenuItem? convertedMenuItem ) in this.ConvertEmptyMenus( dictionary, b, bi ) )
                 {
-                    toReturn.Add(converted.Key, converted.Value);
+                    dictionary.TryAdd( convertedMenuBarItem, convertedMenuItem );
                 }
             }
         }
 
-        return toReturn;
+        return dictionary;
     }
 
     /// <summary>
@@ -137,12 +168,12 @@ public Dictionary<MenuBarItem, MenuItem> ConvertEmptyMenus()
     /// <param name="added">The result of the conversion (same text, same index etc but
     /// <see cref="MenuItem"/> instead of <see cref="MenuBarItem"/>).</param>
     /// <returns><see langword="true"/> if conversion was possible (menu was empty and belonged to tracked menu).</returns>
-    internal static bool ConvertMenuBarItemToRegularItemIfEmpty(MenuBarItem bar, out MenuItem? added)
+    internal static bool ConvertMenuBarItemToRegularItemIfEmpty( MenuBarItem bar, [NotNullWhen( true )] out MenuItem? added )
     {
         added = null;
 
         // bar still has more children so don't convert
-        if (bar.Children.Any())
+        if ( bar.Children.Length != 0 )
         {
             return false;
         }
@@ -152,48 +183,40 @@ internal static bool ConvertMenuBarItemToRegularItemIfEmpty(MenuBarItem bar, out
             return false;
         }
 
-        var children = parent.Children.ToList<MenuItem>();
-        var idx = children.IndexOf(bar);
+        int idx = Array.IndexOf( parent.Children, bar );
 
         if (idx < 0)
         {
             return false;
         }
 
         // bar has no children so convert to MenuItem
-        added = new MenuItem { Title = bar.Title };
-        added.Data = bar.Data;
-        added.Shortcut = bar.Shortcut;
-
-        children.RemoveAt(idx);
-        children.Insert(idx, added);
-
-        parent.Children = children.ToArray();
+        parent.Children[ idx ] = added = new( )
+        {
+            Title = bar.Title,
+            Data = bar.Data,
+            Shortcut = bar.Shortcut
+        };
 
         return true;
     }
 
     /// <inheritdoc cref="ConvertEmptyMenus()"/>
-    private Dictionary<MenuBarItem, MenuItem> ConvertEmptyMenus(MenuBar bar, MenuBarItem mbi)
+    private Dictionary<MenuBarItem, MenuItem> ConvertEmptyMenus(Dictionary<MenuBarItem,MenuItem> dictionary, MenuBar bar, MenuBarItem mbi)
     {
-        var toReturn = new Dictionary<MenuBarItem, MenuItem>();
-
         foreach (var c in mbi.Children.OfType<MenuBarItem>())
         {
-            this.ConvertEmptyMenus(bar, c);
-            if ( ConvertMenuBarItemToRegularItemIfEmpty( c, out var added))
+            this.ConvertEmptyMenus(dictionary,bar, c);
+            if ( ConvertMenuBarItemToRegularItemIfEmpty( c, out MenuItem? added))
             {
-                if (added != null)
-                {
-                    toReturn.Add(c, added);
-                }
+                dictionary.TryAdd( c, added );
 
                 bar.CloseMenu();
                 bar.OpenMenu();
             }
         }
 
-        return toReturn;
+        return dictionary;
     }
 
     private void MenuClosing(object? sender, MenuClosingEventArgs obj)
@@ -204,7 +227,7 @@ private void MenuClosing(object? sender, MenuClosingEventArgs obj)
     private void MenuOpened(object? sender, MenuOpenedEventArgs obj)
     {
         this.CurrentlyOpenMenuItem = obj.MenuItem;
-        this.ConvertEmptyMenus();
+        this.ConvertEmptyMenus( );
     }
 
     private void MenuAllClosed(object? sender, EventArgs e)

src/Operations/MenuOperations/RemoveMenuItemOperation.cs

@@ -64,10 +64,12 @@ public override void Undo()
             return;
         }
 
-        var children = this.Parent.Children.ToList<MenuItem>();
-
-        children.Insert(this.removedAtIdx, this.OperateOn);
-        this.Parent.Children = children.ToArray();
+        this.Parent.Children =
+        [
+            .. Parent.Children[ .. removedAtIdx ],
+            this.OperateOn,
+            .. Parent.Children[ removedAtIdx .. ]
+        ];
         this.Bar?.SetNeedsDisplay();
 
         // if any MenuBarItem were converted to vanilla MenuItem
@@ -123,12 +125,12 @@ protected override bool DoImpl()
             return false;
         }
 
-        var children = this.Parent.Children.ToList<MenuItem>();
-
-        this.removedAtIdx = Math.Max(0, children.IndexOf(this.OperateOn));
-
-        children.Remove(this.OperateOn);
-        this.Parent.Children = children.ToArray();
+        this.removedAtIdx = Math.Max( 0, Array.IndexOf( Parent.Children, OperateOn ) );
+        this.Parent.Children =
+        [
+            .. Parent.Children[ ..removedAtIdx ],
+            .. Parent.Children[ ( removedAtIdx + 1 ).. ]
+        ];
         this.Bar?.SetNeedsDisplay();
 
         if (this.Bar != null)
@@ -137,27 +139,24 @@ protected override bool DoImpl()
         }
 
         // if a top level menu now has no children
-        if (this.Bar != null)
+        var empty = this.Bar?.Menus.Where(bi => bi.Children.Length == 0).ToArray();
+        if (empty?.Any() == true)
         {
-            var empty = this.Bar.Menus.Where(bi => bi.Children.Length == 0).ToArray();
-            if (empty.Any())
-            {
-                // remember where they were
-                this.prunedEmptyTopLevelMenus = empty.ToDictionary(e => Array.IndexOf(this.Bar.Menus, e), v => v);
+            // remember where they were
+            this.prunedEmptyTopLevelMenus = empty.ToDictionary(e => Array.IndexOf(this.Bar.Menus, e), v => v);
 
-                // and remove them
-                this.Bar.Menus = this.Bar.Menus.Except(this.prunedEmptyTopLevelMenus.Values).ToArray();
-            }
+            // and remove them
+            this.Bar.Menus = this.Bar.Menus.Except(this.prunedEmptyTopLevelMenus.Values).ToArray();
+        }
 
-            // if we just removed the last menu header
-            // leaving a completely blank menu bar
-            if (this.Bar.Menus.Length == 0 && this.Bar.SuperView != null)
-            {
-                // remove the bar completely
-                this.Bar.CloseMenu();
-                this.barRemovedFrom = this.Bar.SuperView;
-                this.barRemovedFrom.Remove(this.Bar);
-            }
+        // if we just removed the last menu header
+        // leaving a completely blank menu bar
+        if (this.Bar?.Menus.Length == 0 && this.Bar.SuperView != null)
+        {
+            // remove the bar completely
+            this.Bar.CloseMenu();
+            this.barRemovedFrom = this.Bar.SuperView;
+            this.barRemovedFrom.Remove(this.Bar);
         }
 
         return true;

src/ViewFactory.cs

@@ -28,7 +28,7 @@ public static class ViewFactory
     internal const string DefaultMenuItemText = "Edit Me";
 
     internal static readonly Type[] KnownUnsupportedTypes =
-    {
+    [
         typeof( Toplevel ),
         typeof( Dialog ),
         typeof( FileDialog ),
@@ -42,7 +42,7 @@ public static class ViewFactory
         // BUG These seem to cause stack overflows in CreateSubControlDesigns (see TestAddView_RoundTrip)
         typeof( Wizard ),
         typeof( WizardStep )
-    };
+    ];
 
     /// <summary>
     ///   Gets a new instance of a default <see cref="MenuBarItem" />[], to include as the default initial
@@ -57,12 +57,11 @@ internal static MenuBarItem[] DefaultMenuBarItems
     {
         get
         {
-            return new[]
-            {
-                new MenuBarItem(
-                    "_File (F9)",
-                    new[] { new MenuItem( DefaultMenuItemText, string.Empty, ( ) => { } ) } )
-            };
+            return
+            [
+                new( "_File (F9)",
+                     [ new MenuItem( DefaultMenuItemText, string.Empty, static ( ) => { } ) ] )
+            ];
         }
     }