Merge pull request #363 from ReactiveCocoa/remove-disposable-handle

Remove `CompositeDisposable.DisposableHandle`.

Author: mdiep

Sources/Deprecations+Removals.swift

@@ -75,6 +75,16 @@ extension ComposableMutablePropertyProtocol {
 	public func withValue<Result>(action: (Value) throws -> Result) rethrows -> Result { fatalError() }
 }
 
+extension CompositeDisposable {
+	@available(*, deprecated, message:"Use `Disposable?` instead. The typealias would be removed in a future release.")
+	public typealias DisposableHandle = Disposable?
+}
+
+extension Optional where Wrapped == Disposable {
+	@available(*, unavailable, renamed:"dispose")
+	public func remove() { fatalError() }
+}
+
 @available(*, unavailable, renamed:"SignalProducer.timer")
 public func timer(interval: DispatchTimeInterval, on scheduler: DateScheduler) -> SignalProducer<Date, NoError> { fatalError() }
 
@@ -306,7 +316,7 @@ extension Bag {
 
 extension CompositeDisposable {
 	@available(*, unavailable, renamed:"add(_:)")
-	public func addDisposable(_ d: Disposable) -> DisposableHandle { fatalError() }
+	public func addDisposable(_ d: Disposable) -> Disposable? { fatalError() }
 }
 
 extension Observer {

Sources/Disposable.swift

@@ -110,48 +110,6 @@ public final class CompositeDisposable: Disposable {
 	private let disposables: Atomic<Bag<Disposable>?>
 	private var state: UnsafeAtomicState<DisposableState>
 
-	/// Represents a handle to a disposable previously added to a
-	/// `CompositeDisposable`.
-	///
-	/// - note: `add(_:)` method of `CompositeDisposable` creates instances of
-	///         `DisposableHandle`.
-	public final class DisposableHandle {
-		private var state: UnsafeAtomicState<DisposableState>
-		private var bagToken: Bag<Disposable>.Token?
-		private weak var disposable: CompositeDisposable?
-
-		fileprivate static let empty = DisposableHandle()
-
-		fileprivate init() {
-			self.state = UnsafeAtomicState(.disposed)
-			self.bagToken = nil
-		}
-
-		deinit {
-			state.deinitialize()
-		}
-
-		fileprivate init(bagToken: Bag<Disposable>.Token, disposable: CompositeDisposable) {
-			self.state = UnsafeAtomicState(.active)
-			self.bagToken = bagToken
-			self.disposable = disposable
-		}
-
-		/// Remove the pointed-to disposable from its `CompositeDisposable`.
-		///
-		/// - note: This is useful to minimize memory growth, by removing
-		///         disposables that are no longer needed.
-		public func remove() {
-			if state.tryDispose(), let token = bagToken {
-				_ = disposable?.disposables.modify {
-					$0?.remove(using: token)
-				}
-				bagToken = nil
-				disposable = nil
-			}
-		}
-	}
-
 	public var isDisposed: Bool {
 		return state.is(.disposed)
 	}
@@ -202,41 +160,43 @@ public final class CompositeDisposable: Disposable {
 		}
 	}
 
-	/// Add the given disposable to the list, then return a handle which can
-	/// be used to opaquely remove the disposable later (if desired).
+	/// Add the given disposable to the composite.
 	///
 	/// - parameters:
-	///   - d: Optional disposable.
+	///   - disposable: A disposable.
 	///
-	/// - returns: An instance of `DisposableHandle` that can be used to
-	///            opaquely remove the disposable later (if desired).
+	/// - returns: A disposable to remove `disposable` from the composite. `nil` if the
+	///            composite has been disposed of, `disposable` has been disposed of, or
+	///            `disposable` is `nil`.
 	@discardableResult
-	public func add(_ d: Disposable?) -> DisposableHandle {
-		guard let d = d else {
-			return DisposableHandle.empty
+	public func add(_ disposable: Disposable?) -> Disposable? {
+		guard let d = disposable, !d.isDisposed, !isDisposed else {
+			disposable?.dispose()
+			return nil
 		}
 
-		let handle: DisposableHandle? = disposables.modify {
-			return ($0?.insert(d)).map { DisposableHandle(bagToken: $0, disposable: self) }
-		}
+		return disposables.modify { disposables in
+			guard disposables != nil else { return nil }
 
-		if let handle = handle {
-			return handle
-		} else {
-			d.dispose()
-			return DisposableHandle.empty
+			let token = disposables!.insert(d)
+			return ActionDisposable { [weak self] in
+				self?.disposables.modify {
+					$0?.remove(using: token)
+				}
+			}
 		}
 	}
 
-	/// Add an ActionDisposable to the list.
+	/// Add the given action to the composite.
 	///
 	/// - parameters:
-	///   - action: A closure that will be invoked when `dispose()` is called.
+	///   - action: A closure to be invoked when the composite is disposed of.
 	///
-	/// - returns: An instance of `DisposableHandle` that can be used to
-	///            opaquely remove the disposable later (if desired).
+	/// - returns: A disposable to remove `disposable` from the composite. `nil` if the
+	///            composite has been disposed of, `disposable` has been disposed of, or
+	///            `disposable` is `nil`.
 	@discardableResult
-	public func add(_ action: @escaping () -> Void) -> DisposableHandle {
+	public func add(_ action: @escaping () -> Void) -> Disposable? {
 		return add(ActionDisposable(action: action))
 	}
 
@@ -351,7 +311,7 @@ public final class SerialDisposable: Disposable {
 /// - returns: An instance of `DisposableHandle` that can be used to opaquely
 ///            remove the disposable later (if desired).
 @discardableResult
-public func +=(lhs: CompositeDisposable, rhs: Disposable?) -> CompositeDisposable.DisposableHandle {
+public func +=(lhs: CompositeDisposable, rhs: Disposable?) -> Disposable? {
 	return lhs.add(rhs)
 }
 
@@ -369,7 +329,7 @@ public func +=(lhs: CompositeDisposable, rhs: Disposable?) -> CompositeDisposabl
 /// - returns: An instance of `DisposableHandle` that can be used to opaquely
 ///            remove the disposable later (if desired).
 @discardableResult
-public func +=(lhs: CompositeDisposable, rhs: @escaping () -> ()) -> CompositeDisposable.DisposableHandle {
+public func +=(lhs: CompositeDisposable, rhs: @escaping () -> ()) -> Disposable? {
 	return lhs.add(rhs)
 }
 
@@ -387,7 +347,7 @@ public func +=(lhs: CompositeDisposable, rhs: @escaping () -> ()) -> CompositeDi
 /// - returns: An instance of `DisposableHandle` that can be used to opaquely
 ///            remove the disposable later (if desired).
 @discardableResult
-public func +=(lhs: ScopedDisposable<CompositeDisposable>, rhs: Disposable?) -> CompositeDisposable.DisposableHandle {
+public func +=(lhs: ScopedDisposable<CompositeDisposable>, rhs: Disposable?) -> Disposable? {
 	return lhs.inner.add(rhs)
 }
 
@@ -405,6 +365,6 @@ public func +=(lhs: ScopedDisposable<CompositeDisposable>, rhs: Disposable?) ->
 /// - returns: An instance of `DisposableHandle` that can be used to opaquely
 ///            remove the disposable later (if desired).
 @discardableResult
-public func +=(lhs: ScopedDisposable<CompositeDisposable>, rhs: @escaping () -> ()) -> CompositeDisposable.DisposableHandle {
+public func +=(lhs: ScopedDisposable<CompositeDisposable>, rhs: @escaping () -> ()) -> Disposable? {
 	return lhs.inner.add(rhs)
 }

Sources/Flatten.swift

@@ -452,7 +452,7 @@ extension Signal where Value: SignalProducerProtocol, Error == Value.Error {
 					signal.observe { event in
 						switch event {
 						case .completed, .interrupted:
-							handle.remove()
+							handle?.dispose()
 
 							let shouldComplete: Bool = state.modify { state in
 								state.activeCount -= 1
@@ -839,7 +839,7 @@ extension Signal where Value: SignalProducerProtocol, Error == Value.Error {
 
 						// Dispose all running innerSignals except winning one.
 						if !relayDisposable.isDisposed {
-							disposableHandle.remove()
+							disposableHandle?.dispose()
 							relayDisposable.dispose()
 						}
 

Tests/ReactiveSwiftTests/DisposableSpec.swift

@@ -74,8 +74,8 @@ class DisposableSpec: QuickSpec {
 				let handle = disposable += simpleDisposable
 
 				// We should be allowed to call this any number of times.
-				handle.remove()
-				handle.remove()
+				handle?.dispose()
+				handle?.dispose()
 				expect(simpleDisposable.isDisposed) == false
 
 				disposable.dispose()