Skip to content

Commit

Permalink
Make changes to Action's isExecuting and isEnabled atomic
Browse files Browse the repository at this point in the history
This unifies isExecuting and userEnabled in a single struct, so that
changes to either value can be made atomically.

Changes no longer need to be synchronised by a serial dispatch queue,
and atomically enforcing the enabledness of the action is much simpler.
  • Loading branch information
sharplet committed Sep 27, 2016
1 parent 07ea7e7 commit 20523bc
Showing 1 changed file with 32 additions and 33 deletions.
65 changes: 32 additions & 33 deletions Sources/Action.swift
Original file line number Diff line number Diff line change
Expand Up @@ -31,34 +31,12 @@ public final class Action<Input, Output, Error: Swift.Error> {
public let errors: Signal<Error, NoError>

/// Whether the action is currently executing.
public var isExecuting: Property<Bool> {
return Property(_isExecuting)
}

private let _isExecuting: MutableProperty<Bool> = MutableProperty(false)
public let isExecuting: Property<Bool>

/// Whether the action is currently enabled.
public var isEnabled: Property<Bool> {
return Property(_isEnabled)
}

private let _isEnabled: MutableProperty<Bool> = MutableProperty(false)

/// Whether the instantiator of this action wants it to be enabled.
private let isUserEnabled: Property<Bool>
public let isEnabled: Property<Bool>

/// This queue is used for read-modify-write operations on the `_executing`
/// property.
private let executingQueue = DispatchQueue(
label: "org.reactivecocoa.ReactiveSwift.Action.executingQueue",
attributes: []
)

/// Whether the action should be enabled for the given combination of user
/// enabledness and executing status.
private static func shouldBeEnabled(userEnabled: Bool, executing: Bool) -> Bool {
return userEnabled && !executing
}
private let state: MutableProperty<ActionState>

/// Initializes an action that will be conditionally enabled, and creates a
/// SignalProducer for each input.
Expand All @@ -70,16 +48,24 @@ public final class Action<Input, Output, Error: Swift.Error> {
/// calling `apply(Input)` on the action.
public init<P: PropertyProtocol>(enabledIf property: P, _ execute: @escaping (Input) -> SignalProducer<Output, Error>) where P.Value == Bool {
executeClosure = execute
isUserEnabled = Property(property)

(events, eventsObserver) = Signal<Event<Output, Error>, NoError>.pipe()

values = events.map { $0.value }.skipNil()
errors = events.map { $0.error }.skipNil()

_isEnabled <~ property.producer
.combineLatest(with: isExecuting.producer)
.map(Action.shouldBeEnabled)
state = MutableProperty(ActionState(isExecuting: false, userEnabled: property.value))

property.signal
.take(during: state.lifetime)
.observeValues { [weak state] isEnabled in
state?.modify {
$0.userEnabled = isEnabled
}
}

isEnabled = state.map { $0.isEnabled }
isExecuting = state.map { $0.isExecuting }
}

/// Initializes an action that will be enabled by default, and creates a
Expand Down Expand Up @@ -111,9 +97,9 @@ public final class Action<Input, Output, Error: Swift.Error> {
return SignalProducer { observer, disposable in
var executedProducer: SignalProducer<Output, Error>?

self.executingQueue.sync {
if self._isEnabled.value {
self._isExecuting.value = true
self.state.modify { state in
if state.isEnabled {
state.isExecuting = true
executedProducer = self.executeClosure(input)
}
}
Expand All @@ -133,12 +119,25 @@ public final class Action<Input, Output, Error: Swift.Error> {
}

disposable += {
self._isExecuting.value = false
self.state.modify {
$0.isExecuting = false
}
}
}
}
}

private struct ActionState {
var isExecuting: Bool
var userEnabled: Bool

/// Whether the action should be enabled for the given combination of user
/// enabledness and executing status.
fileprivate var isEnabled: Bool {
return userEnabled && !isExecuting
}
}

public protocol ActionProtocol {
/// The type of argument to apply the action to.
associatedtype Input
Expand Down

0 comments on commit 20523bc

Please sign in to comment.