Skip to content

Minimize NSObject inheritance #2352

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Apr 2, 2020
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions MapboxCoreNavigation/EndOfRouteFeedback.swift
Original file line number Diff line number Diff line change
@@ -3,7 +3,7 @@ import Foundation
/**
Feedback Model Object for End Of Route Experience.
*/
open class EndOfRouteFeedback: NSObject {
open class EndOfRouteFeedback {
/**
Rating: The user's rating for the route. Normalized between 0 and 100.
*/
@@ -17,7 +17,6 @@ open class EndOfRouteFeedback: NSObject {
@nonobjc public init(rating: Int? = nil, comment: String? = nil) {
self.rating = rating
self.comment = comment
super.init()
}
public convenience init(rating ratingNumber: NSNumber?, comment: String?) {
let rating = ratingNumber?.intValue
93 changes: 57 additions & 36 deletions MapboxCoreNavigation/NavigationSettings.swift
Original file line number Diff line number Diff line change
@@ -7,25 +7,53 @@ import Foundation

To specify criteria when calculating routes, use the `NavigationRouteOptions` class. To customize the user experience during a particular turn-by-turn navigation session, use the `NavigationOptions` class when initializing a `NavigationViewController`.
*/
public class NavigationSettings: NSObject {
public class NavigationSettings {

public enum StoredProperty: CaseIterable {
case voiceVolume, voiceMuted, distanceUnit

public var key: String {
switch self {
case .voiceVolume:
return "voiceVolume"
case .voiceMuted:
return "voiceMuted"
case .distanceUnit:
return "distanceUnit"
}
}
}

/**
The volume that the voice controller will use.

This volume is relative to the system’s volume where 1.0 is same volume as the system.
*/
@objc public dynamic var voiceVolume: Float = 1.0
public dynamic var voiceVolume: Float = 1.0 {
didSet {
notifyChanged(property: .voiceVolume, value: voiceVolume)
}
}

/**
Specifies whether to mute the voice controller or not.
*/
@objc public dynamic var voiceMuted : Bool = false
public dynamic var voiceMuted : Bool = false {
didSet {
notifyChanged(property: .voiceMuted, value: voiceMuted)
}
}

/**
Specifies the preferred distance measurement unit.
- note: Anything but `kilometer` and `mile` will fall back to the default measurement for the current locale.
Meters and feets will be used when the presented distances are small enough. See `DistanceFormatter` for more information.
*/
@objc public dynamic var distanceUnit : LengthFormatter.Unit = Locale.current.usesMetric ? .kilometer : .mile
public dynamic var distanceUnit : LengthFormatter.Unit = Locale.current.usesMetric ? .kilometer : .mile {
didSet {
notifyChanged(property: .distanceUnit, value: distanceUnit.rawValue)
}
}

var usesMetric: Bool {
get {
@@ -56,43 +84,36 @@ public class NavigationSettings: NSObject {
})
}()

override init() {
super.init()
for property in properties {
guard let key = property.label else { continue }
let val = UserDefaults.standard.object(forKey: key.prefixed) ?? value(forKey: key)
setValue(val, forKey: key)
addObserver(self, forKeyPath: key, options: .new, context: nil)
}
private func notifyChanged(property: StoredProperty, value: Any) {
UserDefaults.standard.set(value, forKey: property.key.prefixed)
NotificationCenter.default.post(name: .navigationSettingsDidChange,
object: nil,
userInfo: [property.key: value])
}

deinit {
for property in properties {
guard let key = property.label else { continue }
removeObserver(self, forKeyPath: key)
}
}

override public func observeValue(forKeyPath keyPath: String?, of object: Any?, change: [NSKeyValueChangeKey: Any]?, context: UnsafeMutableRawPointer?) {
var found = false

for property in properties {
guard let key = property.label else { continue }
private func setupFromDefaults() {
for property in StoredProperty.allCases {

if key == keyPath {
guard let val = change?[.newKey] else { continue }

UserDefaults.standard.set(val, forKey: key.prefixed)
NotificationCenter.default.post(name: .navigationSettingsDidChange, object: nil, userInfo: [key: val])

found = true
break
guard let val = UserDefaults.standard.object(forKey: property.key.prefixed) else { continue }
switch property {
case .voiceVolume:
if let volume = val as? Float {
voiceVolume = volume
}
case .voiceMuted:
if let muted = val as? Bool {
voiceMuted = muted
}
case .distanceUnit:
if let value = val as? Int, let unit = LengthFormatter.Unit(rawValue: value) {
distanceUnit = unit
}
}
}
if !found {
super.observeValue(forKeyPath: keyPath, of: object, change: change, context: context)
}
}

init() {
setupFromDefaults()
}
}

19 changes: 14 additions & 5 deletions MapboxCoreNavigation/RouteController.swift
Original file line number Diff line number Diff line change
@@ -34,15 +34,16 @@ open class RouteController: NSObject {
}

private var _routeProgress: RouteProgress {
willSet {
resetObservation(for: _routeProgress)
}
didSet {
movementsAwayFromRoute = 0
updateNavigator(with: _routeProgress)
updateObservation(for: _routeProgress)
Copy link
Contributor Author

@Udumft Udumft Mar 17, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't we not only subscribe to legIndex updates here, but also run the subscription handler too to update RouteController with new _routeProgress?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure what you mean by this? _routeProgress is the memoized property that represent's RouteController's copy of the current route progress.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When _routeProgress is set, we observe it's legIndex to call updateRouteLeg when it is updated. But shouldn't we call updateRouteLeg right now, since we've just updated the entire _routeProgress and thus it's legIndex is technically updated too.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Udumft No, it's unnecessary, because the leg is set when we call updateNavigator(with:).

}
}

private var progressObservation: NSKeyValueObservation?

var movementsAwayFromRoute = 0

var routeTask: URLSessionDataTask?
@@ -149,12 +150,20 @@ open class RouteController: NSObject {
updateObservation(for: _routeProgress)
}

deinit {
resetObservation(for: _routeProgress)
}

func resetObservation(for progress: RouteProgress) {
progress.legIndexHandler = nil
}

func updateObservation(for progress: RouteProgress) {
progressObservation = progress.observe(\.legIndex, options: [.old, .new]) { [weak self] (progress, change) in
guard change.newValue != change.oldValue, let legIndex = change.newValue else {
progress.legIndexHandler = { [weak self] (oldValue, newValue) in
guard newValue != oldValue else {
return
}
self?.updateRouteLeg(to: legIndex)
self?.updateRouteLeg(to: newValue)
}
}

16 changes: 9 additions & 7 deletions MapboxCoreNavigation/RouteProgress.swift
Original file line number Diff line number Diff line change
@@ -5,7 +5,7 @@ import Turf
/**
`RouteProgress` stores the user’s progress along a route.
*/
open class RouteProgress: NSObject {
open class RouteProgress {
private static let reroutingAccuracy: CLLocationAccuracy = 90

/**
@@ -16,14 +16,17 @@ open class RouteProgress: NSObject {
/**
Index representing current `RouteLeg`.
*/
@objc dynamic public var legIndex: Int {
public var legIndex: Int {
didSet {
assert(legIndex >= 0 && legIndex < route.legs.endIndex)
// TODO: Set stepIndex to 0 or last index based on whether leg index was incremented or decremented.
currentLegProgress = RouteLegProgress(leg: currentLeg)

legIndexHandler?(oldValue, legIndex)
}
}

typealias LegIndexHandlerAction = (_ oldValue: Int, _ newValue: Int) -> ()
var legIndexHandler: LegIndexHandlerAction?
/**
If waypoints are provided in the `Route`, this will contain which leg the user is on.
*/
@@ -175,7 +178,6 @@ open class RouteProgress: NSObject {
self.route = route
self.legIndex = legIndex
self.currentLegProgress = RouteLegProgress(leg: route.legs[legIndex], stepIndex: 0, spokenInstructionIndex: spokenInstructionIndex)
super.init()

for (legIndex, leg) in route.legs.enumerated() {
var maneuverCoordinateIndex = 0
@@ -272,7 +274,7 @@ open class RouteProgress: NSObject {
/**
`RouteLegProgress` stores the user’s progress along a route leg.
*/
open class RouteLegProgress: NSObject {
open class RouteLegProgress {
/**
Returns the current `RouteLeg`.
*/
@@ -281,7 +283,7 @@ open class RouteLegProgress: NSObject {
/**
Index representing the current step.
*/
@objc public var stepIndex: Int {
public var stepIndex: Int {
didSet {
assert(stepIndex >= 0 && stepIndex < leg.steps.endIndex)
currentStepProgress = RouteStepProgress(step: currentStep)
@@ -515,7 +517,7 @@ open class RouteLegProgress: NSObject {
/**
`RouteStepProgress` stores the user’s progress along a route step.
*/
open class RouteStepProgress: NSObject {
open class RouteStepProgress {
/**
Returns the current `RouteStep`.
*/
6 changes: 2 additions & 4 deletions MapboxNavigation/DataCache.swift
Original file line number Diff line number Diff line change
@@ -1,14 +1,12 @@
import Foundation

public class DataCache: NSObject, BimodalDataCache {
public class DataCache: BimodalDataCache {
let memoryCache: NSCache<NSString, NSData>
let fileCache = FileCache()

public override init() {
public init() {
memoryCache = NSCache<NSString, NSData>()
memoryCache.name = "In-Memory Data Cache"

super.init()

NotificationCenter.default.addObserver(self, selector: #selector(DataCache.clearMemory), name: UIApplication.didReceiveMemoryWarningNotification, object: nil)
}
2 changes: 1 addition & 1 deletion MapboxNavigation/FeedbackItem.swift
Original file line number Diff line number Diff line change
@@ -10,7 +10,7 @@ extension UIImage {
/**
A single feedback item displayed on an instance of `FeedbackViewController`.
*/
public class FeedbackItem: NSObject {
public class FeedbackItem {
/**
The title of feedback item. This will be rendered directly below the image.
*/
28 changes: 13 additions & 15 deletions MapboxNavigation/MapboxVoiceController.swift
Original file line number Diff line number Diff line change
@@ -48,21 +48,6 @@ open class MapboxVoiceController: RouteVoiceController, AVAudioPlayerDelegate {
super.init(navigationService: navigationService)

audioPlayer?.delegate = self

volumeToken = NavigationSettings.shared.observe(\.voiceVolume) { [weak self] (settings, change) in
self?.audioPlayer?.volume = settings.voiceVolume
}

muteToken = NavigationSettings.shared.observe(\.voiceMuted) { [weak self] (settings, change) in
if settings.voiceMuted {
self?.audioPlayer?.stop()

guard let strongSelf = self else { return }
strongSelf.safeUnduckAudio(instruction: nil, engine: self?.speech) {
strongSelf.voiceControllerDelegate?.voiceController(strongSelf, spokenInstructionsDidFailWith: $0)
}
}
}
}

deinit {
@@ -75,6 +60,19 @@ open class MapboxVoiceController: RouteVoiceController, AVAudioPlayerDelegate {
audioPlayer?.delegate = nil
}

@objc override func didUpdateSettings(notification: NSNotification) {
if let isMuted = notification.userInfo?[NavigationSettings.StoredProperty.voiceMuted.key] as? Bool, isMuted {
audioPlayer?.stop()

safeUnduckAudio(instruction: nil, engine: speech) {
voiceControllerDelegate?.voiceController(self, spokenInstructionsDidFailWith: $0)
}
}
if let voiceVolume = notification.userInfo?[NavigationSettings.StoredProperty.voiceVolume.key] as? Float {
audioPlayer?.volume = voiceVolume
}
}

public func audioPlayerDidFinishPlaying(_ player: AVAudioPlayer, successfully flag: Bool) {
safeUnduckAudio(instruction: nil, engine: speech) {
voiceControllerDelegate?.voiceController(self, spokenInstructionsDidFailWith: $0)
6 changes: 3 additions & 3 deletions MapboxNavigation/NavigationOptions.swift
Original file line number Diff line number Diff line change
@@ -8,7 +8,7 @@ import MapboxCoreNavigation

- note: `NavigationOptions` is designed to be used with the `NavigationViewController` class to customize the user experience. To specify criteria when calculating routes, use the `NavigationRouteOptions` class. To modify user preferences that persist across navigation sessions, use the `NavigationSettings` class.
*/
open class NavigationOptions: NSObject, NavigationCustomizable {
open class NavigationOptions: NavigationCustomizable {
/**
The styles that the view controller’s internal `StyleManager` object can select from for display.

@@ -41,8 +41,8 @@ open class NavigationOptions: NSObject, NavigationCustomizable {
open var bottomBanner: ContainerViewController?

// This makes the compiler happy.
required public override init() {
super.init()
required public init() {
// do nothing
}

/**
17 changes: 8 additions & 9 deletions MapboxNavigation/RouteVoiceController.swift
Original file line number Diff line number Diff line change
@@ -78,9 +78,6 @@ open class RouteVoiceController: NSObject, AVSpeechSynthesizerDelegate {
var lastSpokenInstruction: SpokenInstruction?
var routeProgress: RouteProgress?

var volumeToken: NSKeyValueObservation?
var muteToken: NSKeyValueObservation?

/**
Default initializer for `RouteVoiceController`.
*/
@@ -118,18 +115,20 @@ open class RouteVoiceController: NSObject, AVSpeechSynthesizerDelegate {
NotificationCenter.default.addObserver(self, selector: #selector(didPassSpokenInstructionPoint(notification:)), name: .routeControllerDidPassSpokenInstructionPoint, object: service.router)
NotificationCenter.default.addObserver(self, selector: #selector(pauseSpeechAndPlayReroutingDing(notification:)), name: .routeControllerWillReroute, object: service.router)
NotificationCenter.default.addObserver(self, selector: #selector(didReroute(notification:)), name: .routeControllerDidReroute, object: service.router)

muteToken = NavigationSettings.shared.observe(\.voiceMuted) { [weak self] (settings, change) in
if settings.voiceMuted {
self?.speechSynth.stopSpeaking(at: .immediate)
}
}
NotificationCenter.default.addObserver(self, selector: #selector(didUpdateSettings(notification:)), name: .navigationSettingsDidChange, object: NavigationSettings.shared)
}

func suspendNotifications() {
NotificationCenter.default.removeObserver(self, name: .routeControllerDidPassSpokenInstructionPoint, object: nil)
NotificationCenter.default.removeObserver(self, name: .routeControllerWillReroute, object: nil)
NotificationCenter.default.removeObserver(self, name: .routeControllerDidReroute, object: nil)
NotificationCenter.default.removeObserver(self, name: .navigationSettingsDidChange, object: nil)
}

@objc func didUpdateSettings(notification: NSNotification) {
if let isMuted = notification.userInfo?[NavigationSettings.StoredProperty.voiceMuted.key] as? Bool, isMuted {
speechSynth.stopSpeaking(at: .immediate)
}
}

@objc func didReroute(notification: NSNotification) {
16 changes: 10 additions & 6 deletions MapboxNavigation/StyleManager.swift
Original file line number Diff line number Diff line change
@@ -52,7 +52,7 @@ public extension StyleManagerDelegate {
A manager that handles `Style` objects. The manager listens for significant time changes
and changes to the content size to apply an approriate style for the given condition.
*/
open class StyleManager: NSObject {
open class StyleManager {
/**
The receiver of the delegate. See `StyleManagerDelegate` for more information.
*/
@@ -85,6 +85,7 @@ open class StyleManager: NSObject {
}

internal var date: Date?
private var timeOfDayTimer: Timer?

var currentStyleType: StyleType?
private(set) var currentStyle: Style? {
@@ -94,15 +95,14 @@ open class StyleManager: NSObject {
}
}

public override init() {
super.init()
public init() {
resumeNotifications()
resetTimeOfDayTimer()
}

deinit {
suspendNotifications()
NSObject.cancelPreviousPerformRequests(withTarget: self, selector: #selector(timeOfDayChanged), object: nil)
timeOfDayTimer?.invalidate()
}

func resumeNotifications() {
@@ -116,7 +116,7 @@ open class StyleManager: NSObject {
}

func resetTimeOfDayTimer() {
NSObject.cancelPreviousPerformRequests(withTarget: self, selector: #selector(timeOfDayChanged), object: nil)
timeOfDayTimer?.invalidate()

guard automaticallyAdjustsStyleForTimeOfDay && styles.count > 1 else { return }
guard let location = delegate?.location(for:self) else { return }
@@ -132,7 +132,11 @@ open class StyleManager: NSObject {
return
}

perform(#selector(timeOfDayChanged), with: nil, afterDelay: interval+1)
timeOfDayTimer = Timer(timeInterval: interval + 1,
repeats: false,
block: { [weak self] _ in
self?.timeOfDayChanged()
})
}

@objc func preferredContentSizeChanged(_ notification: Notification) {