Merge pull request #511 from campersau/perf1

Use backwards for loop instead of linq for performance in AttributeCollection.Remove
zzzprojects · Aug 1, 2023 · e581d69 · e581d69
2 parents 2e98e14 + 0e4d99e
commit e581d69
Show file tree

Hide file tree

Showing 2 changed files with 20 additions and 9 deletions.
diff --git a/src/HtmlAgilityPack.Shared/HtmlAttributeCollection.cs b/src/HtmlAgilityPack.Shared/HtmlAttributeCollection.cs
@@ -8,7 +8,6 @@
 using System;
 using System.Collections;
 using System.Collections.Generic;
-using System.Linq;
 
 namespace HtmlAgilityPack
 {
@@ -368,20 +367,14 @@ public void Remove(string name)
                 throw new ArgumentNullException("name");
             }
 
-            List<int> listToRemove = new List<int>();
-            for (int i = 0; i < items.Count; i++)
+            for (int i = items.Count - 1; i >= 0; i--)
             {
                 HtmlAttribute att = items[i];
                 if (String.Equals(att.Name, name, StringComparison.OrdinalIgnoreCase))
                 {
-                    listToRemove.Add(i);
+                    RemoveAt(i);
                 }
             }
-
-            foreach(var i in listToRemove.OrderByDescending(x => x))
-			{ 
-                RemoveAt(i);
-            }
         }
 
         /// <summary>

diff --git a/src/Tests/HtmlAgilityPack.Tests.Net45/HtmlDocumentTests.cs b/src/Tests/HtmlAgilityPack.Tests.Net45/HtmlDocumentTests.cs
@@ -564,6 +564,24 @@ public void OuterHtmlHasBeenCalled_RemoveCalled_SubsequentOuterHtmlCallsAreBroke
             Assert.AreEqual("<html><head></head><body></body></html>", doc.DocumentNode.OuterHtml);
         }
 
+        [Test]
+        public void TestRemoveAttribute()
+        {
+            var output = @"<h1>This is new heading</h1>";
+
+            string html = "<h1 a=\"foo\" b=\"bar\" A=\"baz\">This is new heading</h1>";
+
+            var htmlDoc = new HtmlDocument();
+            htmlDoc.LoadHtml(html);
+
+            var h1Node = htmlDoc.DocumentNode.SelectSingleNode("//h1");
+
+            h1Node.Attributes.Remove("a");
+            h1Node.Attributes.Remove("b");
+
+            Assert.AreEqual(h1Node.OuterHtml, output);
+        }
+
         [Test]
         public void TestAttributeDeEntitizeValue()
         {