update

efficient thread safety

Author: saadshams

Sources/PureMVC/core/Controller.swift

@@ -38,38 +38,38 @@ registrations.
 `@see org.puremvc.swift.multicore.patterns.command.MacroCommand MacroCommand`
 */
 open class Controller: IController {
-    
+
     /// Local reference to View
     internal var view: IView!
-    
+
     // Mapping of Notification names to factories that instanties and returns `ICommand` Class instances
     internal var commandMap = [String: () -> ICommand]()
-    
+
     // Concurrent queue for commandMap
     // for speed and convenience of running concurrently while reading, and thread safety of blocking while mutating
     internal let commandMapQueue = DispatchQueue(label: "org.puremvc.controller.commandMapQueue", attributes: DispatchQueue.Attributes.concurrent)
-    
+
     /// The Multiton Key for this app
     internal private(set) var multitonKey: String
-    
+
     // The Multiton Controller instanceMap.
     private static var instanceMap = [String: IController]()
-    
+
     // instance Queue for thread safety
     private static let instanceQueue = DispatchQueue(label: "org.puremvc.controller.instanceQueue", attributes: DispatchQueue.Attributes.concurrent)
-    
+
     /// Message constant
     internal static let MULTITON_MSG = "Controller instance for this Multiton key already constructed!"
-    
+
     /**
     Constructor.
-    
+
     This `IController` implementation is a Multiton,
     so you should not call the constructor
     directly, but instead call the static Factory method,
     passing the unique key for this instance
     `Controller.getInstance( multitonKey )`
-    
+
     @throws Error if instance for this Multiton key has already been constructed
     */
     public init(key: String) {
@@ -78,17 +78,17 @@ open class Controller: IController {
         Controller.instanceMap[key] = self
         initializeController()
     }
-    
+
     /**
     Initialize the Multiton `Controller` instance.
-    
+
     Called automatically by the constructor.
-    
+
     Note that if you are using a subclass of `View`
     in your application, you should *also* subclass `Controller`
     and override the `initializeController` method in the
     following way:
-    
+
         // ensure that the Controller is talking to my IView implementation
         override public func initializeController(  ) {
             view = MyView.getInstance(multitonKey) { MyView(key: self.multitonKey) }
@@ -97,10 +97,10 @@ open class Controller: IController {
     open func initializeController() {
         view = View.getInstance(multitonKey) { key in View(key: key) }
     }
-    
+
     /**
     `Controller` Multiton Factory method.
-    
+
     - parameter key: multitonKey
     - parameter factory: reference that returns `IController`
     - returns: the Multiton instance
@@ -113,18 +113,18 @@ open class Controller: IController {
         }
         return instanceMap[key]!
     }
-    
+
     /**
     Register a particular `ICommand` class as the handler
     for a particular `INotification`.
-    
+
     If an `ICommand` has already been registered to
     handle `INotification`s with this name, it is no longer
     used, the new `ICommand` is used instead.
-    
+
     The Observer for the new ICommand is only created if this the
     first time an ICommand has been regisered for this Notification name.
-    
+
     - parameter notificationName: the name of the `INotification`
     - parameter factory: reference that returns `ICommand`
     */
@@ -136,60 +136,59 @@ open class Controller: IController {
             commandMap[notificationName] = factory
         }
     }
-    
+
     /**
     If an `ICommand` has previously been registered
     to handle a the given `INotification`, then it is executed.
-    
-    - parameter note: an `INotification`
+
+    - parameter notification: an `INotification`
     */
     open func executeCommand(_ notification: INotification) {
-        commandMapQueue.sync {
-            if let factory = commandMap[notification.name] {
-                let commandInstance = factory()
-                commandInstance.initializeNotifier(multitonKey)
-                commandInstance.execute(notification)
-            }
+        let factory = commandMapQueue.sync {
+            commandMap[notification.name]
         }
+        
+        guard let command = factory?() else { return }
+        
+        command.initializeNotifier(multitonKey)
+        command.execute(notification)
     }
-    
+
     /**
     Check if a Command is registered for a given Notification
-    
+
     - parameter notificationName:
     - returns: whether a Command is currently registered for the given `notificationName`.
     */
     open func hasCommand(_ notificationName: String) -> Bool {
-        var result = false
         commandMapQueue.sync {
-            result = commandMap[notificationName] != nil
+            commandMap[notificationName] != nil
         }
-        return result
     }
-    
+
     /**
     Remove a previously registered `ICommand` to `INotification` mapping.
-    
+
     - parameter notificationName: the name of the `INotification` to remove the `ICommand` mapping for
     */
     open func removeCommand(_ notificationName: String) {
-        if hasCommand(notificationName) {
-            commandMapQueue.sync(flags: .barrier) {
-                view.removeObserver(notificationName, notifyContext: self)
-                commandMap.removeValue(forKey: notificationName)
-            }
+        commandMapQueue.sync(flags: .barrier) {
+            guard commandMap[notificationName] != nil else { return }
+            view.removeObserver(notificationName, notifyContext: self)
+            commandMap.removeValue(forKey: notificationName)
         }
-    }
 
+    }
+
     /**
     Remove an IController instance
-    
+
     - parameter multitonKey: of IController instance to remove
     */
     open class func removeController(_ key: String) {
         instanceQueue.sync(flags: .barrier) {
             _ = instanceMap.removeValue(forKey: key)
         }
     }
-    
+
 }

Sources/PureMVC/core/Model.swift

@@ -103,11 +103,11 @@ open class Model: IModel {
     - parameter proxy: an `IProxy` to be held by the `Model`.
     */
     open func registerProxy(_ proxy: IProxy) {
+        proxy.initializeNotifier(multitonKey)
         proxyMapQueue.sync(flags: .barrier) {
-            proxy.initializeNotifier(multitonKey)
             proxyMap[proxy.name] = proxy
-            proxy.onRegister()
         }
+        proxy.onRegister()
     }
 
     /**
@@ -117,11 +117,9 @@ open class Model: IModel {
     - returns: the `IProxy` instance previously registered with the given `proxyName`.
     */
     open func retrieveProxy(_ proxyName: String) -> IProxy? {
-        var proxy: IProxy?
         proxyMapQueue.sync {
-            proxy = proxyMap[proxyName]
+            proxyMap[proxyName]
         }
-        return proxy
     }
 
     /**
@@ -131,11 +129,9 @@ open class Model: IModel {
     - returns: whether a Proxy is currently registered with the given `proxyName`.
     */
     open func hasProxy(_ proxyName: String) -> Bool {
-        var result = false
         proxyMapQueue.sync {
-            result = proxyMap[proxyName] != nil
+            proxyMap[proxyName] != nil
         }
-        return result
     }
 
     /**
@@ -147,11 +143,10 @@ open class Model: IModel {
     @discardableResult open func removeProxy(_ proxyName: String) -> IProxy? {
         var removed: IProxy?
         proxyMapQueue.sync(flags: .barrier) {
-            if let proxy = proxyMap[proxyName] {
-                proxy.onRemove()
-                removed = proxyMap.removeValue(forKey: proxyName)
-            }
+            removed = proxyMap.removeValue(forKey: proxyName)
         }
+        
+        removed?.onRemove()
         return removed
     }