Conversation
| /// | ||
| /// Defaults to 1 second. |
There was a problem hiding this comment.
The 'default' comment here I think is not necessary. If the default is changed later, since it's not set here, we'll forget about it and have conflicting comments.
| self.pressedDuration = pressedDuration | ||
| } | ||
|
|
||
| public func blinky(_ blinky: Blinky, didChangeLedState isOn: Bool) { |
There was a problem hiding this comment.
This should be marked as open instead of public. The difference is, open will allow subclasses to override from outside the module it's defined in, so users of CoreBluetooth-Mock library. I think this is the intended behaviour.
Source: Swift Documentation
|
|
||
| public func blinky(_ blinky: Blinky, didChangeButtonSubscriptionState isSubscribed: Bool) { | ||
| DispatchQueue.main.async { [weak self] in | ||
| guard let self = self else { return } |
There was a problem hiding this comment.
guard let self else { return }
Is enough.
| // First fire after `period`, then repeat every `period`. | ||
| t.schedule(deadline: .now() + period, repeating: period) | ||
| t.setEventHandler { [weak self, weak blinky] in | ||
| guard let self = self, let blinky = blinky, self.timer != nil else { return } |
There was a problem hiding this comment.
guard let self, let blinky, let timer else { return } (Perhaps let timer = self.timer might be needed instead)
Easier to read.
| guard let t = timer else { return } | ||
| t.setEventHandler {} | ||
| t.cancel() | ||
| timer = nil |
There was a problem hiding this comment.
defer {
self.timer = nil
}
guard let timer else { return }
timer.setEventHandler {}
timer.cancel()
| .build() | ||
|
|
||
| // Handle state changes. | ||
| weak let userDelegate = delegate |
There was a problem hiding this comment.
This should be made a weak property instead of living on the stack.
| var ledStateDidChange: ((Bool) -> ())! | ||
| var buttonSubscriptionStateDidChange: ((Bool) -> ())! |
There was a problem hiding this comment.
Please redesign:
- These properties should not be forced optionals.
- Alternate proposal: private protocol that Blinky class implements. Then have a weak pointer to this delegate in BlinkyImpl so the implementation stays private or internal.
2 participants
This PR adds a bit of more flexibility to the sample
Blinkyspec, added in #165.It also replaces the mock implementation in
MockPeripherals.swiftto use the one from the library.