HiveBrain v1.2.0
Get Started
← Back to all entries
principleswiftMinor

Observer design pattern in Swift

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
swiftobserverdesignpattern

Problem

The problem:

We have a set of mobile mechanics who are assigned specific zip codes. Each zip code has its own hourly rate. We want to increase and decrease these rates when the number of idle mechanics within a zip code falls or goes above specific thresholds. This way we can proactively set the going rate for each mechanics when demand is high and bring it down when demand is low within a specific zip code.

The solution:

I implemented a solution using the Observer Design Pattern, however I want to make sure I got the terminology right, I think what I call subscriber in my solution is what is called an observer and what I call an observer is actually a handler that propagates the notifications.

More importantly I also want to make sure nothing I've done violates the pattern's definition.

Either way it's been awhile since I dealt with design patterns so any input will be greatly appreciated. Here is the code: Reading through a complicated implementation of a design pattern could seem a bit daunting here so if you're interested the repo as I mentioned earlier can be found here

```
import Foundation

class Mechanic{

var observer: Observer?

let name: String
var zipcode: Zipcode

var status: Status = .Idle{
didSet{
observer?.propertyChanged("Status", oldValue: oldValue.rawValue, newValue: status.rawValue, options: ["Zipcode": zipcode.value])
}
}

init(name: String, location: Zipcode){
self.name = name
self.zipcode = location
}
}

enum Status: Int{
case Idle = 1, OnTheWay, Busy
}

protocol Observer: class{
var subscribers: [Subscriber] {get set}

func propertyChanged(propertyName: String, oldValue: Int, newValue: Int, options: [String:String]?)

func subscribe(subscriber: Subscriber)

func unsubscribe(subscriber: Subscriber)
}

class MechanicObserver: Observer{

var subscribers: [Subscriber] = []

func propertyChanged(propertyName: String, oldValue: Int, newValue: Int, options:[String:String]?){
print("Change

Solution

protocol Observer: class{
  var subscribers: [Subscriber] {get set}

  func propertyChanged(propertyName: String, oldValue: Int, newValue: Int, options: [String:String]?)

  func subscribe(subscriber: Subscriber)

  func unsubscribe(subscriber: Subscriber)
}


Nevermind your terminology, there's a major problem here. Who needs to know all those things? The Mechanic class which is notifying the observer only needs to know that the propertyChanged method exists. And the rest of what the observer does with that property changed information is up to the observer.

Maybe it has subscribers that can subscribe and unsubscribe via these methods. Maybe it has a delegate. Maybe it runs some closures. Maybe it just runs its own code. Maybe it posts an NSNotification. It really doesn't matter. So with that in mind, the subscribers array and the subscribe and unsubscribe methods should be removed from the protocol.

Now, with that in mind, I think that MechanicObserver class is a really unnecessary man in the middle here. This seems over complicated. The Mechanic tells the MechanicObserver who tells the MechanicObserver subscribers. And problematically, whether or not those subscribers get notified is based on whether or not they have a particular string in a special array property they are forced to have? There are several issues here.

First of all, again, I don't see the point in having the MechanicObserver rather than just having subscribers subscribe directly to the Mechanic.

Second, you only allow subscribing to changes in Int properties, and only allow string data in the options. You've really boxed yourself in very tight with the protocol, just one reason this is going to have some scalability issues.

Third, this looping string comparison can quickly become very expensive if we have several things subscribed to this change and they each have several values in their subscribers array.

Finally, this quickly becomes very problematic when ZipCodePriceManager wants to subscribe to the status of mechanics... and then you apply the same pattern to your TowTruck model class and your Garage model class, and they also have a status property, and probably a zipCode too... and who knows what else. There is no way to indicate what model type or even what instance of that model actually had the changing property.

Code Snippets

protocol Observer: class{
  var subscribers: [Subscriber] {get set}

  func propertyChanged(propertyName: String, oldValue: Int, newValue: Int, options: [String:String]?)

  func subscribe(subscriber: Subscriber)

  func unsubscribe(subscriber: Subscriber)
}

Context

StackExchange Code Review Q#125289, answer score: 2

Revisions (0)

No revisions yet.