Add queue support and keypath support to ObserverSet

[Jon889]

We currently have some extension methods that add support for specifying the queue that callback is called, this integrates it more nicely and makes some other changes.

Also I changed the indentation to use tabs instead of spaces as it's more accessible.

I'll update the Example app if this is acceptable. We use it extensively in the Tesco App and after these changes all our Unit tests pass.

[Jon889]

I was going to name with ObserverSetEntry and have the above class called AnyObserverSetEntry, but naming it this way is more backwards compatible.

[jsanderson44]

Could we name this something like DefaultObserverSetEntry instead? Impl doesn't read very well in my opinion

[Jon889]

I'm not 100% sure about this but it makes sense that this shouldn't be @discardableResult to prompt the user to remember to remove the observer at some point. If they really want it to be always added then they can do _ = observerSet.add(...) which I've had to do in some places in the Tesco App where we set up observers on app launch.

[Jon889]

Added this function for convenience when you want to use a closure but want to tie to the lifecycle of an object.

One use case is instead of this:

let someObserverSet = ObserverSet<Int>()
let someObserverSet = ObserverSet<String>()

class Foo {
  private let observer1: Any
  private let observer2: Any
  init() {
      // Use closures to ignore the parameters that are different
      observer1 = someObserverSet.add { [weak self] _ in self?.update() }
      observer2 = anotherObserverSet.add { [weak self] _ in self?.update() }
  }
  func update() {
  
  }
  deinit {
    someObserverSet.remove(observer1)
    someObserverSet.remove(observer2)
  }
}

Just do this:

class Foo {
  init() {
      // Use closures to ignore the parameters that are different
      someObserverSet.add(self) { [weak self] _ in self?.update() }
      anotherObserverSet.add(self) { [weak self] _ in self?.update() }
  }
  func update() {
  
  }
}

An alternative would be to allow observers that have no parameters for any ObserverSet that has a non void Parameters type, but that would remove some of the type safety.

class Foo {
  init() {
      // Use closures to ignore the parameters that are different
      someObserverSet.add(self, Foo.update)
      anotherObserverSet.add(self, Foo.update) // oops I meant to do Foo.bar which takes the parameter but there is no error because any () -> Void function can be used as a callback for any ObserverSet
  }
  func update() {
  
  }

  func bar(_: String) { }
}

[Jon889]

I switched to removeAll(where:) instead of filter because it reads better

[Jon889]

This isn't needed for the other adds as Swift seems to realise () -> Void and (Void) -> Void are the same thing

[jsanderson44]

Do you think its worth putting an assertionFailure in here to indicate that it should be subclassed and overridden?

assertionFailure("Function should be overridden before being called")

[jsanderson44]

Very minor. But might be cleaner to guard around observer rather than have these nester if let statements.

Suggested change

	if let observer = observer {
	let callBack = self.callBack(observer)
	if let queue = queue {
	queue.async {
	callBack(parameters)
	}
	} else {
	callBack(parameters)
	}
	}
	guard let observer = observer else { return }
	let callBack = self.callBack(observer)
	if let queue = queue {
	queue.async {
	callBack(parameters)
	}
	} else {
	callBack(parameters)
	}

[jsanderson44]

Hey @Jon889. Sorry it's taken so long for someone to review this. It looks good from my perspective, just a couple of minor suggestions. Ive asked a couple of others to review as well before we merge.

[jsanderson44]

The current swift version for this repo is 4.0 so these one line implicit returns wont work yet.

Suggested change

	fileprivate var isExpired: Bool { false }
	fileprivate var isExpired: Bool { return false }

[jsanderson44]

As above

Suggested change

	override var isExpired: Bool { observer == nil }
	override var isExpired: Bool { return observer == nil }

[roger-tan]

Hi @Jon889 , Your PR looks great. is there a chance to get the Unit Testing fixed?

To fix the issue with Xcode 12, I updated the initializers of TestObserver with the code below that and this should resolve the issue. It might also help to migrate to Swift 5 in future.

init(observee: TestObservee) {
    observee.voidObservers.add(self, voidSent)
    observee.stringObservers.add(self, stringChanged)
    observee.twoStringObservers.add(self, twoStringChanged)
    observee.intObservers.add(self, intChanged)
    observee.intAndStringObservers.add(self, intAndStringChanged)
}