Merge pull request #379 from Reactive-Extensions/MiscDisposableImprovements

Misc. improvements to Disposable types

Rx.NET/Source/src/System.Reactive/Disposables/CancellationDisposable.cs

@@ -42,10 +42,7 @@ namespace System.Reactive.Disposables
         /// <summary>
         /// Cancels the underlying <seealso cref="CancellationTokenSource"/>.
         /// </summary>
-        public void Dispose()
-        {
-            _cts.Cancel();
-        }
+        public void Dispose() => _cts.Cancel();
 
         /// <summary>
         /// Gets a value that indicates whether the object is disposed.

Rx.NET/Source/src/System.Reactive/Disposables/CompositeDisposable.cs

@@ -2,6 +2,7 @@
 // The .NET Foundation licenses this file to you under the Apache 2.0 License.
 // See the LICENSE file in the project root for more information. 
 
+using System.Collections;
 using System.Collections.Generic;
 using System.Linq;
 
@@ -45,8 +46,8 @@ namespace System.Reactive.Disposables
         /// Initializes a new instance of the <see cref="CompositeDisposable"/> class from a group of disposables.
         /// </summary>
         /// <param name="disposables">Disposables that will be disposed together.</param>
-        /// <exception cref="ArgumentNullException"><paramref name="disposables"/> is null.</exception>
-        /// <exception cref="ArgumentException">Any of the disposables in the <paramref name="disposables"/> collection is null.</exception>
+        /// <exception cref="ArgumentNullException"><paramref name="disposables"/> is <c>null</c>.</exception>
+        /// <exception cref="ArgumentException">Any of the disposables in the <paramref name="disposables"/> collection is <c>null</c>.</exception>
         public CompositeDisposable(params IDisposable[] disposables)
             : this((IEnumerable<IDisposable>)disposables)
         {
@@ -56,21 +57,21 @@ namespace System.Reactive.Disposables
         /// Initializes a new instance of the <see cref="CompositeDisposable"/> class from a group of disposables.
         /// </summary>
         /// <param name="disposables">Disposables that will be disposed together.</param>
-        /// <exception cref="ArgumentNullException"><paramref name="disposables"/> is null.</exception>
-        /// <exception cref="ArgumentException">Any of the disposables in the <paramref name="disposables"/> collection is null.</exception>
+        /// <exception cref="ArgumentNullException"><paramref name="disposables"/> is <c>null</c>.</exception>
+        /// <exception cref="ArgumentException">Any of the disposables in the <paramref name="disposables"/> collection is <c>null</c>.</exception>
         public CompositeDisposable(IEnumerable<IDisposable> disposables)
         {
             if (disposables == null)
                 throw new ArgumentNullException(nameof(disposables));
 
             _disposables = new List<IDisposable>(disposables);
-            
+
             //
             // Doing this on the list to avoid duplicate enumeration of disposables.
             //
             if (_disposables.Contains(null))
                 throw new ArgumentException(Strings_Core.DISPOSABLES_CANT_CONTAIN_NULL, nameof(disposables));
-            
+
             _count = _disposables.Count;
         }
 
@@ -83,7 +84,7 @@ namespace System.Reactive.Disposables
         /// Adds a disposable to the <see cref="CompositeDisposable"/> or disposes the disposable if the <see cref="CompositeDisposable"/> is disposed.
         /// </summary>
         /// <param name="item">Disposable to add.</param>
-        /// <exception cref="ArgumentNullException"><paramref name="item"/> is null.</exception>
+        /// <exception cref="ArgumentNullException"><paramref name="item"/> is <c>null</c>.</exception>
         public void Add(IDisposable item)
         {
             if (item == null)
@@ -99,8 +100,11 @@ namespace System.Reactive.Disposables
                     _count++;
                 }
             }
+
             if (shouldDispose)
+            {
                 item.Dispose();
+            }
         }
 
         /// <summary>
@@ -108,7 +112,7 @@ namespace System.Reactive.Disposables
         /// </summary>
         /// <param name="item">Disposable to remove.</param>
         /// <returns>true if found; false otherwise.</returns>
-        /// <exception cref="ArgumentNullException"><paramref name="item"/> is null.</exception>
+        /// <exception cref="ArgumentNullException"><paramref name="item"/> is <c>null</c>.</exception>
         public bool Remove(IDisposable item)
         {
             if (item == null)
@@ -140,15 +144,21 @@ namespace System.Reactive.Disposables
                             _disposables = new List<IDisposable>(_disposables.Capacity / 2);
 
                             foreach (var d in old)
+                            {
                                 if (d != null)
+                                {
                                     _disposables.Add(d);
+                                }
+                            }
                         }
                     }
                 }
             }
 
             if (shouldDispose)
+            {
                 item.Dispose();
+            }
 
             return shouldDispose;
         }
@@ -173,8 +183,9 @@ namespace System.Reactive.Disposables
             if (currentDisposables != null)
             {
                 foreach (var d in currentDisposables)
-                    if (d != null)
-                        d.Dispose();
+                {
+                    d?.Dispose();
+                }
             }
         }
 
@@ -192,8 +203,9 @@ namespace System.Reactive.Disposables
             }
 
             foreach (var d in currentDisposables)
-                if (d != null)
-                    d.Dispose();
+            {
+                d?.Dispose();
+            }
         }
 
         /// <summary>
@@ -201,7 +213,7 @@ namespace System.Reactive.Disposables
         /// </summary>
         /// <param name="item">Disposable to search for.</param>
         /// <returns>true if the disposable was found; otherwise, false.</returns>
-        /// <exception cref="ArgumentNullException"><paramref name="item"/> is null.</exception>
+        /// <exception cref="ArgumentNullException"><paramref name="item"/> is <c>null</c>.</exception>
         public bool Contains(IDisposable item)
         {
             if (item == null)
@@ -218,7 +230,7 @@ namespace System.Reactive.Disposables
         /// </summary>
         /// <param name="array">Array to copy the contained disposables to.</param>
         /// <param name="arrayIndex">Target index at which to copy the first disposable of the group.</param>
-        /// <exception cref="ArgumentNullException"><paramref name="array"/> is null.</exception>
+        /// <exception cref="ArgumentNullException"><paramref name="array"/> is <c>null</c>.</exception>
         /// <exception cref="ArgumentOutOfRangeException"><paramref name="arrayIndex"/> is less than zero. -or - <paramref name="arrayIndex"/> is larger than or equal to the array length.</exception>
         public void CopyTo(IDisposable[] array, int arrayIndex)
         {
@@ -258,7 +270,7 @@ namespace System.Reactive.Disposables
         /// Returns an enumerator that iterates through the <see cref="CompositeDisposable"/>.
         /// </summary>
         /// <returns>An enumerator to iterate over the disposables.</returns>
-        System.Collections.IEnumerator System.Collections.IEnumerable.GetEnumerator() => GetEnumerator();
+        IEnumerator IEnumerable.GetEnumerator() => GetEnumerator();
 
         /// <summary>
         /// Gets a value that indicates whether the object is disposed.

Rx.NET/Source/src/System.Reactive/Disposables/MultipleAssignmentDisposable.cs

@@ -37,17 +37,19 @@ namespace System.Reactive.Disposables
         /// <summary>
         /// Gets or sets the underlying disposable. After disposal, the result of getting this property is undefined.
         /// </summary>
-        /// <remarks>If the MutableDisposable has already been disposed, assignment to this property causes immediate disposal of the given disposable object.</remarks>
+        /// <remarks>If the <see cref="MultipleAssignmentDisposable"/> has already been disposed, assignment to this property causes immediate disposal of the given disposable object.</remarks>
         public IDisposable Disposable
         {
             get
             {
                 var a = Volatile.Read(ref _current);
+
                 // Don't leak the DISPOSED sentinel
                 if (a == BooleanDisposable.True)
                 {
                     a = DefaultDisposable.Instance;
                 }
+
                 return a;
             }
 
@@ -55,6 +57,7 @@ namespace System.Reactive.Disposables
             {
                 // Let's read the current value atomically (also prevents reordering).
                 var old = Volatile.Read(ref _current);
+
                 for (;;)
                 {
                     // If it is the disposed instance, dispose the value.
@@ -63,13 +66,16 @@ namespace System.Reactive.Disposables
                         value?.Dispose();
                         return;
                     }
+
                     // Atomically swap in the new value and get back the old.
                     var b = Interlocked.CompareExchange(ref _current, value, old);
+
                     // If the old and new are the same, the swap was successful and we can quit
                     if (old == b)
                     {
                         return;
                     }
+
                     // Otherwise, make the old reference the current and retry.
                     old = b;
                 }
@@ -83,11 +89,13 @@ namespace System.Reactive.Disposables
         {
             // Read the current atomically.
             var a = Volatile.Read(ref _current);
+
             // If it is the disposed instance, don't bother further.
             if (a != BooleanDisposable.True)
             {
                 // Atomically swap in the disposed instance.
                 a = Interlocked.Exchange(ref _current, BooleanDisposable.True);
+
                 // It is possible there was a concurrent Dispose call so don't need to call Dispose()
                 // on DISPOSED
                 if (a != BooleanDisposable.True)

Rx.NET/Source/src/System.Reactive/Disposables/RefCountDisposable.cs

@@ -96,8 +96,7 @@ namespace System.Reactive.Disposables
                 }
             }
 
-            if (disposable != null)
-                disposable.Dispose();
+            disposable?.Dispose();
         }
 
         private void Release()
@@ -122,11 +121,10 @@ namespace System.Reactive.Disposables
                 }
             }
 
-            if (disposable != null)
-                disposable.Dispose();
+            disposable?.Dispose();
         }
 
-        sealed class InnerDisposable : IDisposable
+        private sealed class InnerDisposable : IDisposable
         {
             private RefCountDisposable _parent;
 
@@ -137,9 +135,7 @@ namespace System.Reactive.Disposables
 
             public void Dispose()
             {
-                var parent = Interlocked.Exchange(ref _parent, null);
-                if (parent != null)
-                    parent.Release();
+                Interlocked.Exchange(ref _parent, null)?.Release();
             }
         }
     }

Rx.NET/Source/src/System.Reactive/Disposables/ScheduledDisposable.cs

@@ -46,7 +46,9 @@ namespace System.Reactive.Disposables
                 var current = _disposable;
 
                 if (current == BooleanDisposable.True)
+                {
                     return DefaultDisposable.Instance; // Don't leak the sentinel value.
+                }
 
                 return current;
             }
@@ -60,10 +62,7 @@ namespace System.Reactive.Disposables
         /// <summary>
         /// Disposes the wrapped disposable on the provided scheduler.
         /// </summary>
-        public void Dispose()
-        {
-            Scheduler.Schedule(DisposeInner);
-        }
+        public void Dispose() => Scheduler.Schedule(DisposeInner);
 
         private void DisposeInner()
         {

Rx.NET/Source/src/System.Reactive/Disposables/SerialDisposable.cs

@@ -58,10 +58,13 @@ namespace System.Reactive.Disposables
                         _current = value;
                     }
                 }
-                if (old != null)
-                    old.Dispose();
-                if (shouldDispose && value != null)
-                    value.Dispose();
+
+                old?.Dispose();
+
+                if (shouldDispose)
+                {
+                    value?.Dispose();
+                }
             }
         }
 
@@ -82,8 +85,7 @@ namespace System.Reactive.Disposables
                 }
             }
 
-            if (old != null)
-                old.Dispose();
+            old?.Dispose();
         }
     }
 }

Rx.NET/Source/src/System.Reactive/Disposables/SingleAssignmentDisposable.cs

@@ -38,7 +38,7 @@ namespace System.Reactive.Disposables
         /// <summary>
         /// Gets or sets the underlying disposable. After disposal, the result of getting this property is undefined.
         /// </summary>
-        /// <exception cref="InvalidOperationException">Thrown if the SingleAssignmentDisposable has already been assigned to.</exception>
+        /// <exception cref="InvalidOperationException">Thrown if the <see cref="SingleAssignmentDisposable"/> has already been assigned to.</exception>
         public IDisposable Disposable
         {
             get
@@ -46,7 +46,9 @@ namespace System.Reactive.Disposables
                 var current = _current;
 
                 if (current == BooleanDisposable.True)
+                {
                     return DefaultDisposable.Instance; // Don't leak the sentinel value.
+                }
 
                 return current;
             }
@@ -60,8 +62,7 @@ namespace System.Reactive.Disposables
                 if (old != BooleanDisposable.True)
                     throw new InvalidOperationException(Strings_Core.DISPOSABLE_ALREADY_ASSIGNED);
 
-                if (value != null)
-                    value.Dispose();
+                value?.Dispose();
             }
         }
 
@@ -70,9 +71,7 @@ namespace System.Reactive.Disposables
         /// </summary>
         public void Dispose()
         {
-            var old = Interlocked.Exchange(ref _current, BooleanDisposable.True);
-            if (old != null)
-                old.Dispose();
+            Interlocked.Exchange(ref _current, BooleanDisposable.True)?.Dispose();
         }
     }
 }

Rx.NET/Source/src/System.Reactive/Disposables/StableCompositeDisposable.cs

@@ -67,7 +67,7 @@ namespace System.Reactive.Disposables
             get;
         }
 
-        class Binary : StableCompositeDisposable
+        private sealed class Binary : StableCompositeDisposable
         {
             private volatile IDisposable _disposable1;
             private volatile IDisposable _disposable2;
@@ -82,21 +82,12 @@ namespace System.Reactive.Disposables
 
             public override void Dispose()
             {
-                var old1 = Interlocked.Exchange(ref _disposable1, null);
-                if (old1 != null)
-                {
-                    old1.Dispose();
-                }
-
-                var old2 = Interlocked.Exchange(ref _disposable2, null);
-                if (old2 != null)
-                {
-                    old2.Dispose();
-                }
+                Interlocked.Exchange(ref _disposable1, null)?.Dispose();
+                Interlocked.Exchange(ref _disposable2, null)?.Dispose();
             }
         }
 
-        class NAry : StableCompositeDisposable
+        private sealed class NAry : StableCompositeDisposable
         {
             private volatile List<IDisposable> _disposables;