Add TryAdd, Fix Add to throw on existing keys in dictionaries

Author: xPaw

ValveKeyValue/ValveKeyValue.Test/EdgeCaseTestCase.cs

@@ -431,6 +431,56 @@ public void AddStringKVObjectAddsNamedChild()
             Assert.That((string)obj["key1"], Is.EqualTo("value1"));
         }
 
+        [Test]
+        public void AddThrowsOnDuplicateKeyInDictCollection()
+        {
+            var obj = KVObject.Collection();
+            obj.Add("key", "value1");
+
+            Assert.That(() => obj.Add("key", "value2"), Throws.ArgumentException);
+        }
+
+        [Test]
+        public void AddAllowsDuplicateKeyInListCollection()
+        {
+            var obj = KVObject.ListCollection();
+            obj.Add("key", "value1");
+            obj.Add("key", "value2");
+
+            Assert.That(obj.Count, Is.EqualTo(2));
+        }
+
+        [Test]
+        public void TryAddReturnsFalseOnDuplicateKeyInDictCollection()
+        {
+            var obj = KVObject.Collection();
+            obj.Add("key", "value1");
+
+            Assert.That(obj.TryAdd("key", "value2"), Is.False);
+            Assert.That(obj.Count, Is.EqualTo(1));
+            Assert.That((string)obj["key"], Is.EqualTo("value1"));
+        }
+
+        [Test]
+        public void TryAddReturnsTrueOnNewKeyInDictCollection()
+        {
+            var obj = KVObject.Collection();
+
+            Assert.That(obj.TryAdd("key", "value"), Is.True);
+            Assert.That(obj.Count, Is.EqualTo(1));
+            Assert.That((string)obj["key"], Is.EqualTo("value"));
+        }
+
+        [Test]
+        public void TryAddAlwaysReturnsTrueForListCollection()
+        {
+            var obj = KVObject.ListCollection();
+            obj.Add("key", "value1");
+
+            Assert.That(obj.TryAdd("key", "value2"), Is.True);
+            Assert.That(obj.Count, Is.EqualTo(2));
+        }
+
         #endregion
 
         #region CreateArray from KVObject enumerable

ValveKeyValue/ValveKeyValue.Test/Test Data/apisurface.txt

@@ -84,6 +84,7 @@ public class ValveKeyValue.KVDocument
     public ushort ToUInt16(IFormatProvider provider);
     public uint ToUInt32(IFormatProvider provider);
     public ulong ToUInt64(IFormatProvider provider);
+    public bool TryAdd(string key, ValveKeyValue.KVObject value);
     public bool TryGetValue(string name, out ValveKeyValue.KVObject& child);
 }
 
@@ -239,6 +240,7 @@ public class ValveKeyValue.KVObject
     public ushort ToUInt16(IFormatProvider provider);
     public uint ToUInt32(IFormatProvider provider);
     public ulong ToUInt64(IFormatProvider provider);
+    public bool TryAdd(string key, ValveKeyValue.KVObject value);
     public bool TryGetValue(string name, out ValveKeyValue.KVObject& child);
 }
 

ValveKeyValue/ValveKeyValue/KVObject.cs

@@ -192,7 +192,7 @@ public KVObject this[string key]
             set
             {
                 ArgumentNullException.ThrowIfNull(key);
-                SetChild(key, value);
+                TryInsert(key, value ?? Null(), InsertionBehavior.OverwriteExisting);
             }
         }
 
@@ -296,22 +296,25 @@ public bool ContainsKey(string name)
 
         /// <summary>
         /// Adds a named child to this collection.
+        /// For dictionary-backed collections, throws if the key already exists.
+        /// For list-backed collections, always appends (duplicate keys are allowed).
         /// </summary>
+        /// <exception cref="ArgumentException">The key already exists in a dictionary-backed collection.</exception>
         public void Add(string key, KVObject value)
         {
             ArgumentNullException.ThrowIfNull(key);
+            TryInsert(key, value, InsertionBehavior.ThrowOnExisting);
+        }
 
-            switch (_ref)
-            {
-                case Dictionary<string, KVObject> dict:
-                    dict[key] = value;
-                    break;
-                case List<KeyValuePair<string, KVObject>> list when ValueType == KVValueType.Collection:
-                    list.Add(new KeyValuePair<string, KVObject>(key, value));
-                    break;
-                default:
-                    throw new InvalidOperationException($"Cannot add a named child to a {ValueType} value.");
-            }
+        /// <summary>
+        /// Tries to add a named child to this collection.
+        /// Returns <c>false</c> if the key already exists in a dictionary-backed collection.
+        /// For list-backed collections, always succeeds (duplicate keys are allowed).
+        /// </summary>
+        public bool TryAdd(string key, KVObject value)
+        {
+            ArgumentNullException.ThrowIfNull(key);
+            return TryInsert(key, value, InsertionBehavior.None);
         }
 
         /// <summary>
@@ -500,48 +503,66 @@ public ReadOnlySpan<byte> AsSpan()
         internal List<KVObject> GetArrayList()
             => (List<KVObject>)_ref;
 
-        internal Dictionary<string, KVObject> GetCollectionDict()
-            => (Dictionary<string, KVObject>)_ref;
-
-        internal List<KeyValuePair<string, KVObject>> GetCollectionList()
-            => (List<KeyValuePair<string, KVObject>>)_ref;
-
         #endregion
 
         #region Private helpers
 
-        private void SetChild(string key, KVObject value)
+        private enum InsertionBehavior
         {
-            value ??= Null();
+            None,
+            OverwriteExisting,
+            ThrowOnExisting,
+        }
 
+        private bool TryInsert(string key, KVObject value, InsertionBehavior behavior)
+        {
             switch (_ref)
             {
                 case Dictionary<string, KVObject> dict:
-                    dict[key] = value;
-                    break;
-                case List<KeyValuePair<string, KVObject>> list when ValueType == KVValueType.Collection:
-                    var firstIndex = list.FindIndex(c => c.Key == key);
-                    if (firstIndex >= 0)
+                    if (behavior == InsertionBehavior.OverwriteExisting)
                     {
-                        list[firstIndex] = new KeyValuePair<string, KVObject>(key, value);
+                        dict[key] = value;
+                        return true;
+                    }
+
+                    if (dict.TryAdd(key, value))
+                    {
+                        return true;
+                    }
+
+                    if (behavior == InsertionBehavior.ThrowOnExisting)
+                    {
+                        throw new ArgumentException($"An item with the same key has already been added. Key: {key}");
+                    }
 
-                        // Remove any remaining duplicates after the replaced entry
-                        for (var i = list.Count - 1; i > firstIndex; i--)
+                    return false;
+
+                case List<KeyValuePair<string, KVObject>> list when ValueType == KVValueType.Collection:
+                    if (behavior == InsertionBehavior.OverwriteExisting)
+                    {
+                        var firstIndex = list.FindIndex(c => c.Key == key);
+                        if (firstIndex >= 0)
                         {
-                            if (list[i].Key == key)
+                            list[firstIndex] = new KeyValuePair<string, KVObject>(key, value);
+
+                            // Remove any remaining duplicates after the replaced entry
+                            for (var i = list.Count - 1; i > firstIndex; i--)
                             {
-                                list.RemoveAt(i);
+                                if (list[i].Key == key)
+                                {
+                                    list.RemoveAt(i);
+                                }
                             }
+
+                            return true;
                         }
                     }
-                    else
-                    {
-                        list.Add(new KeyValuePair<string, KVObject>(key, value));
-                    }
 
-                    break;
+                    list.Add(new KeyValuePair<string, KVObject>(key, value));
+                    return true;
+
                 default:
-                    throw new InvalidOperationException($"Cannot set a child on a {ValueType} value.");
+                    throw new InvalidOperationException($"Cannot insert a named child into a {ValueType} value.");
             }
         }