diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/Headers/CacheControlHeaderValue.cs b/src/libraries/System.Net.Http/src/System/Net/Http/Headers/CacheControlHeaderValue.cs index e73adeb9b2bc86..f5bf79362bc64a 100644 --- a/src/libraries/System.Net.Http/src/System/Net/Http/Headers/CacheControlHeaderValue.cs +++ b/src/libraries/System.Net.Http/src/System/Net/Http/Headers/CacheControlHeaderValue.cs @@ -52,17 +52,7 @@ public bool NoCache set { _noCache = value; } } - public ICollection NoCacheHeaders - { - get - { - if (_noCacheHeaders == null) - { - _noCacheHeaders = new ObjectCollection(s_checkIsValidToken); - } - return _noCacheHeaders; - } - } + public ICollection NoCacheHeaders => _noCacheHeaders ??= new ObjectCollection(s_checkIsValidToken); public bool NoStore { @@ -124,17 +114,7 @@ public bool Private set { _privateField = value; } } - public ICollection PrivateHeaders - { - get - { - if (_privateHeaders == null) - { - _privateHeaders = new ObjectCollection(s_checkIsValidToken); - } - return _privateHeaders; - } - } + public ICollection PrivateHeaders => _privateHeaders ??= new ObjectCollection(s_checkIsValidToken); public bool MustRevalidate { @@ -148,17 +128,7 @@ public bool ProxyRevalidate set { _proxyRevalidate = value; } } - public ICollection Extensions - { - get - { - if (_extensions == null) - { - _extensions = new ObjectCollection(); - } - return _extensions; - } - } + public ICollection Extensions => _extensions ??= new ObjectCollection(); public CacheControlHeaderValue() { @@ -604,11 +574,7 @@ private static bool TrySetOptionalTokenList(NameValueHeaderValue nameValue, ref return false; } - if (destination == null) - { - destination = new ObjectCollection(s_checkIsValidToken); - } - + destination ??= new ObjectCollection(s_checkIsValidToken); destination.Add(valueString.Substring(current, tokenLength)); current = current + tokenLength; diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/Headers/ContentDispositionHeaderValue.cs b/src/libraries/System.Net.Http/src/System/Net/Http/Headers/ContentDispositionHeaderValue.cs index 5eb614cee8f394..57a4bd8ca0ccc9 100644 --- a/src/libraries/System.Net.Http/src/System/Net/Http/Headers/ContentDispositionHeaderValue.cs +++ b/src/libraries/System.Net.Http/src/System/Net/Http/Headers/ContentDispositionHeaderValue.cs @@ -40,17 +40,7 @@ public string DispositionType } } - public ICollection Parameters - { - get - { - if (_parameters == null) - { - _parameters = new ObjectCollection(); - } - return _parameters; - } - } + public ICollection Parameters => _parameters ??= new ObjectCollection(); public string? Name { diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/Headers/MediaTypeHeaderValue.cs b/src/libraries/System.Net.Http/src/System/Net/Http/Headers/MediaTypeHeaderValue.cs index 260e30208a7153..36651d68a984ac 100644 --- a/src/libraries/System.Net.Http/src/System/Net/Http/Headers/MediaTypeHeaderValue.cs +++ b/src/libraries/System.Net.Http/src/System/Net/Http/Headers/MediaTypeHeaderValue.cs @@ -55,17 +55,7 @@ public string? CharSet } } - public ICollection Parameters - { - get - { - if (_parameters == null) - { - _parameters = new ObjectCollection(); - } - return _parameters; - } - } + public ICollection Parameters => _parameters ??= new ObjectCollection(); [DisallowNull] public string? MediaType diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/Headers/NameValueWithParametersHeaderValue.cs b/src/libraries/System.Net.Http/src/System/Net/Http/Headers/NameValueWithParametersHeaderValue.cs index e7f58ea0932bdb..3c624ab7693407 100644 --- a/src/libraries/System.Net.Http/src/System/Net/Http/Headers/NameValueWithParametersHeaderValue.cs +++ b/src/libraries/System.Net.Http/src/System/Net/Http/Headers/NameValueWithParametersHeaderValue.cs @@ -18,17 +18,7 @@ public class NameValueWithParametersHeaderValue : NameValueHeaderValue, ICloneab private ObjectCollection? _parameters; - public ICollection Parameters - { - get - { - if (_parameters == null) - { - _parameters = new ObjectCollection(); - } - return _parameters; - } - } + public ICollection Parameters => _parameters ??= new ObjectCollection(); public NameValueWithParametersHeaderValue(string name) : base(name) diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/Headers/ObjectCollection.cs b/src/libraries/System.Net.Http/src/System/Net/Http/Headers/ObjectCollection.cs index bb6ef3b328c6b2..5809b7c5a27b78 100644 --- a/src/libraries/System.Net.Http/src/System/Net/Http/Headers/ObjectCollection.cs +++ b/src/libraries/System.Net.Http/src/System/Net/Http/Headers/ObjectCollection.cs @@ -2,57 +2,212 @@ // The .NET Foundation licenses this file to you under the MIT license. // See the LICENSE file in the project root for more information. +using System.Collections; using System.Collections.Generic; -using System.Collections.ObjectModel; using System.Diagnostics; namespace System.Net.Http.Headers { - // We need to prevent 'null' values in the collection. Since List allows them, we will create - // a custom collection class. It is less efficient than List but only used for small collections. - internal sealed class ObjectCollection : Collection where T : class + /// An list that prohibits null elements and that is optimized for a small number of elements. + [DebuggerDisplay("Count = {Count}")] + [DebuggerTypeProxy(nameof(DebugView))] + internal sealed class ObjectCollection : ICollection where T : class { - private static readonly Action s_defaultValidator = CheckNotNull; + private const int DefaultSize = 4; - private readonly Action _validator; + /// Optional delegate used to validate added items. + private readonly Action? _validator; + /// null, a T, or a T[]. + internal object? _items; + /// Number of elements stored in the collection. + internal int _size; - public ObjectCollection() - : this(s_defaultValidator) + public ObjectCollection() { } + + public ObjectCollection(Action validator) => _validator = validator; + + public int Count => _size; + + public bool IsReadOnly => false; + + public void Add(T item) { + // Validate the item, either just by checking it for null, or using a custom validator, + // which should also check for null. + if (_validator is null) + { + if (item is null) + { + throw new ArgumentNullException(nameof(item)); + } + } + else + { + _validator.Invoke(item); + Debug.Assert(item != null); + } + + if (_items is null) + { + // The collection is empty. Just store the new item directly. + _items = item; + _size = 1; + } + else if (_items is T existingItem) + { + // The collection has a single item stored directly. Upgrade to + // an array, and store both the existing and new items. + Debug.Assert(_size == 1); + T[] items = new T[DefaultSize]; + items[0] = existingItem; + items[1] = item; + _items = items; + _size = 2; + } + else + { + T[] array = (T[])_items; + int size = _size; + if ((uint)size < (uint)array.Length) + { + // There's room in the existing array. Add the item. + array[size] = item; + } + else + { + // We need to grow the array. Do so, and store the new item. + Debug.Assert(_size > 0); + Debug.Assert(_size == array.Length); + + var newItems = new T[array.Length * 2]; + Array.Copy(array, newItems, size); + _items = newItems; + newItems[size] = item; + } + _size = size + 1; + } } - public ObjectCollection(Action validator) - : base(new List()) + public void Clear() { - Debug.Assert(validator != null, $"{nameof(validator)} must not be null."); - _validator = validator; + _items = null; + _size = 0; } - // This is only used internally to enumerate the collection - // without the enumerator allocation. - public new List.Enumerator GetEnumerator() + public bool Contains(T item) => + ReferenceEquals(item, _items) || + (_size != 0 && _items is T[] items && Array.IndexOf(items, item, 0, _size) != -1); + + public void CopyTo(T[] array, int arrayIndex) { - return ((List)Items).GetEnumerator(); + if (_items is T[] items) + { + Array.Copy(items, 0, array, arrayIndex, _size); + } + else + { + Debug.Assert(_size == 0 || _size == 1); + if (array is null || _size > array.Length - arrayIndex) + { + // Use Array.CopyTo to throw the right exceptions. + new T[] { (T)_items! }.CopyTo(array!, arrayIndex); + } + else if (_size == 1) + { + array[arrayIndex] = (T)_items!; + } + } } - protected override void InsertItem(int index, T item) + public bool Remove(T item) { - _validator(item); - base.InsertItem(index, item); + if (ReferenceEquals(_items, item)) + { + _items = null; + _size = 0; + return true; + } + + if (_items is T[] items) + { + int index = Array.IndexOf(items, item, 0, _size); + if (index != -1) + { + _size--; + if (index < _size) + { + Array.Copy(items, index + 1, items, index, _size - index); + } + items[_size] = null!; + + return true; + } + } + + return false; } - protected override void SetItem(int index, T item) + public Enumerator GetEnumerator() => new Enumerator(this); + IEnumerator IEnumerable.GetEnumerator() => GetEnumerator(); + IEnumerator IEnumerable.GetEnumerator() => GetEnumerator(); + + public struct Enumerator : IEnumerator { - _validator(item); - base.SetItem(index, item); + private readonly ObjectCollection _list; + private int _index; + private T _current; + + internal Enumerator(ObjectCollection list) + { + _list = list; + _index = 0; + _current = default!; + } + + public void Dispose() { } + + public bool MoveNext() + { + ObjectCollection list = _list; + + if ((uint)_index < (uint)list._size) + { + _current = list._items is T[] items ? items[_index] : (T)list._items!; + _index++; + return true; + } + + _index = _list._size + 1; + _current = default!; + return false; + } + + public T Current => _current!; + + object? IEnumerator.Current => _current; + + void IEnumerator.Reset() + { + _index = 0; + _current = default!; + } } - private static void CheckNotNull(T item) + internal sealed class DebugView { - // Null values cannot be added to the collection. - if (item == null) + private readonly ObjectCollection _collection; + + public DebugView(ObjectCollection collection) => _collection = collection ?? throw new ArgumentNullException(nameof(collection)); + + [DebuggerBrowsable(DebuggerBrowsableState.RootHidden)] + public T[] Items { - throw new ArgumentNullException(nameof(item)); + get + { + T[] items = new T[_collection.Count]; + _collection.CopyTo(items, 0); + return items; + } } } } diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/Headers/RangeHeaderValue.cs b/src/libraries/System.Net.Http/src/System/Net/Http/Headers/RangeHeaderValue.cs index b9528755a1f926..a6b80ac851c168 100644 --- a/src/libraries/System.Net.Http/src/System/Net/Http/Headers/RangeHeaderValue.cs +++ b/src/libraries/System.Net.Http/src/System/Net/Http/Headers/RangeHeaderValue.cs @@ -25,17 +25,7 @@ public string Unit } } - public ICollection Ranges - { - get - { - if (_ranges == null) - { - _ranges = new ObjectCollection(); - } - return _ranges; - } - } + public ICollection Ranges => _ranges ??= new ObjectCollection(); public RangeHeaderValue() { diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/Headers/TransferCodingHeaderValue.cs b/src/libraries/System.Net.Http/src/System/Net/Http/Headers/TransferCodingHeaderValue.cs index 22daaea0d11335..c833b894d45499 100644 --- a/src/libraries/System.Net.Http/src/System/Net/Http/Headers/TransferCodingHeaderValue.cs +++ b/src/libraries/System.Net.Http/src/System/Net/Http/Headers/TransferCodingHeaderValue.cs @@ -21,17 +21,7 @@ public string Value get { return _value; } } - public ICollection Parameters - { - get - { - if (_parameters == null) - { - _parameters = new ObjectCollection(); - } - return _parameters; - } - } + public ICollection Parameters => _parameters ??= new ObjectCollection(); internal TransferCodingHeaderValue() { diff --git a/src/libraries/System.Net.Http/tests/UnitTests/Headers/ObjectCollectionTest.cs b/src/libraries/System.Net.Http/tests/UnitTests/Headers/ObjectCollectionTest.cs index 2b9a1ac293f391..a48ed76026cb55 100644 --- a/src/libraries/System.Net.Http/tests/UnitTests/Headers/ObjectCollectionTest.cs +++ b/src/libraries/System.Net.Http/tests/UnitTests/Headers/ObjectCollectionTest.cs @@ -2,7 +2,6 @@ // The .NET Foundation licenses this file to you under the MIT license. // See the LICENSE file in the project root for more information. -using System; using System.Net.Http.Headers; using Xunit; @@ -18,14 +17,13 @@ public void Ctor_ExecuteBothOverloads_MatchExpectation() ObjectCollection c = new ObjectCollection(); c.Add("value1"); - c.Insert(0, "value2"); + c.Add("value2"); Assert.Throws(() => { c.Add(null); }); - Assert.Throws(() => { c[0] = null; }); Assert.Equal(2, c.Count); - Assert.Equal("value2", c[0]); - Assert.Equal("value1", c[1]); + Assert.True(c.Contains("value2")); + Assert.True(c.Contains("value1")); // Use custom validator c = new ObjectCollection(item => @@ -37,13 +35,11 @@ public void Ctor_ExecuteBothOverloads_MatchExpectation() }); c.Add("value1"); - c[0] = "value2"; Assert.Throws(() => { c.Add(null); }); - Assert.Throws(() => { c[0] = null; }); Assert.Equal(1, c.Count); - Assert.Equal("value2", c[0]); + Assert.True(c.Contains("value1")); } } }