artman
Proposal: Read Only Signal
@artman, would you be supportive of adding a readOnly Signal? When exposing a Signal from a subsystem, it is difficult to reason about who might be calling fire on a given Signal because the fire method is public. It would be desirable for certain systems to expose a readonly Signal to better enforce when a Signal may be fired.
Two ideas on how to accomplish this:
Option 1: Create a new ReadOnlySignalBox which hides the fireable Signal internally.
class ReadOnlySignalBox<T> {
public typealias SignalCallback = (T) -> Void
// MARK: - Private Properties
private let boxedSignal: Signals.Signal<T>
init(boxedSignal: Signals.Signal<T>) {
self.boxedSignal = boxedSignal
}
// MARK: - Public API
func subscribe(on observer: AnyObject, callback: @escaping Signals.Signal<T>.SignalCallback) -> Signals.SignalSubscription<T> {
return boxedSignal.subscribe(on: observer, callback: callback)
}
/// Other subscribe + cancel methods
}
Option 2: Create a new protocol SignalObservingProtocol (feel free to bike shed on naming) Which Signal conforms to by default.
public protocol SignalObservingProtocol {
associatedtype T
typealias ObservableCallback = (T) -> Void
func subscribe(on observer: AnyObject, callback: @escaping ObservableCallback) -> Signals.SignalSubscription<T>
/// Other subscribe + cancel methods
}
extension Signals.Signal: SignalObservingProtocol {}
/// And provide a type erased `AnySignalObservingProtocol` with a type erased AnySignalObservingProtocol for convenience.
class AnySignalObservingProtocol<T>: SignalObservingProtocol {
func subscribe(on observer: AnyObject, callback: @escaping (T) -> Void) -> Signals.SignalSubscription<T> {
return _subscribe(observer, callback)
}
init<S: ReadOnlyObservableProtocol>(readOnlyObservable: S) where S.T == T {
_subscribe = { observer, callback in
return readOnlyObservable.subscribe(on: observer, callback: callback)
}
}
private var _subscribe: (AnyObject, @escaping (T) -> Void) -> Signals.SignalSubscription<T>
}
Thoughts?
Yeah, I've never liked the fact that the fire method is exposed to potential observers. However, I also would't like to overcomplicate the construction of a Signal too much or make the library hard to understand.
I think the best way to achieve this would be to create a new protocol - Signalling - which only contains the subscription methods. Signal would conform to Signalling and users would be able to have a private reference to the instance while only exposing Signalling publicly.
The project I’m working on now has implemented something like. We described our Signalling protocol as a Protocol with Associated Type (PAT) in order to keep the generic T (DataType here for clarity). e.g:
protocol Signalling {
associatedtype DataType
public func subscribeOnce(with observer: AnyObject, callback: @escaping SignalCallback) -> SignalSubscription<DataType>
/// Other public methods except for `fire(:)`
}
The issue this creates is that now we can no longer hold on to a raw Signalling type because the compiler can’t lock down the PAT. We would then be forcing each consumer of the library to provide concrete implementation of the full Signalling protocol with the associatedtype DataType locked down. Or we introduce type erasure and have an AnySignalling type which type erases the protocol.
final class AnySignalling<T>: Signalling {
typealias T
private let _subscribeOnce: (AnyObject, SignallingCallback) -> SignalSubscription<DataType>
init<U: Signalling>(signalling: U) where U.DataType == T {
_subscribeOnce = U.subscribeOnce
}
public func subscribeOnce(with observer: AnyObject, callback: @escaping SignalCallback) -> SignalSubscription<DataType> {
return _subscribeOnce(observer, callback)
}
/// Other Signalling methods omitted..
}
/// Now since the interfaces match exactly `Signal` can conform to `Signalling` for free.
extension Signal: Signalling {}
The type erasure approach has some obvious downsides, now consumers of the library would be encouraged to switch from exposing a Signal to expose an AnySignal. However there wouldn’t be any breaking change because Signal would still be public and without any interface changes.
In order to adopt the benefits of encapsulating the fire(:) method on Signal and only exposing a public Signalling consumers would need to jump through some hoops. For one, there would now need to be two properties for every Signal. The first which is the privateSignal which the type uses to encapsulate thefire(:)and the second, the type erasedAnySignal`.
class MyManager {
private let privateSignal = Signal<Bool>()
public let subscribableBool: AnySignalling<Bool>
init() {
subscribableBool = AnySignalling<Bool>(signalling: privateSignal)
}
func foo() {
privateSignal.fire(true)
}
}
We have something similar on my current project which pulls in this library. While the setup is a bit onerous, we have found the encapsulation benefits worth the extra grunt work. Curious what your thoughts are.