Observer pattern using a set of observers and delegated properties

Question

The idea is to implement the observer pattern, with a separate notification method for each observed property of the observed object. Let me start with a simplified use case:

class Point {
    val observers: ObserverSet<PointObserver>()
    val x by observers.observe(0f, PointObserver::xChanged)
    val y by observers.observe(0f, PointObserver::yChanged)
}

interface PointObserver {
    fun xChanged(old: Float, new: Float)
    fun yChanged(old: Float, new: Float)
}

class PointView(point: Point) {
    init {
        point.observers.add(object : PointObserver {
            override fun xChanged(old: Float, new: Float) { ... }
            override fun yChanged(old: Float, new: Float) { ... }
        })
    }
}

I'm new to Kotlin, so I'd like some feedback on the following implementation:

import kotlin.properties.ReadWriteProperty
import kotlin.reflect.KProperty

/**
 * A set of observers for a particular object, to easily implement the Observer Pattern without needing inheritance.
 *
 * This can be made a public property of the object. Call [observe] to create delegated properties for each property
 * that observers need to be notified about.
 *
 * @param O The type of the observers, typically an interface.
 */
class ObserverSet<O> {

    private val observers = mutableSetOf<O>()
    private val properties = mutableListOf<ObservedProperty<*>>()

    /**
     * Sets whether an observer is present in the set. If the observer was actually added to the set, its notification
     * methods will be called with the current value of the property.
     *
     * @param observer The observer.
     * @param present If true, the observer will be added to the set if not already there. If false, the observer will
     *   be removed from the set if it's there.
     */
    fun toggle(observer: O, present: Boolean) {
        if (present) {
            val wasPresent = observer in observers
            observers.add(observer)
            if (!wasPresent) {
                init(observer)
            }
        } else {
            observers.remove(observer)
        }
    }

    /**
     * Creates a delegate property with a callback that notifies each observer.
     *
     * @param V The type of the property.
     * @param initialValue The initial value of the property.
     * @param onChanged Function on the [O] interface that will be called with the old and new value after the
     *   property has been changed.
     */
    fun <V> observe(initialValue: V, onChanged: O.(old: V, new: V) -> Unit): ReadWriteProperty<Any?, V> {
        val property = ObservedProperty(initialValue, onChanged)
        properties.add(property)
        return property
    }

    private fun init(observer: O) {
        properties.forEach { property ->
            property.initObserver(observer)
        }
    }

    private inner class ObservedProperty<V>(initialValue: V, private val onChanged: O.(old: V, new: V) -> Unit) :
            ReadWriteProperty<Any?, V> {

        private var value = initialValue

        override fun getValue(thisRef: Any?, property: KProperty<*>): V {
            return value
        }

        override fun setValue(thisRef: Any?, property: KProperty<*>, value: V) {
            val oldValue = this.value
            this.value = value
            observers.forEach { observer ->
                observer.onChanged(oldValue, value)
            }
        }

        fun initObserver(observer: O) {
            observer.onChanged(value, value)
        }
    }
}

(The toggle method is the only one I need right now; regular add and remove could of course be implemented as well.)

I'm mainly interested in efficiency considerations, ways to make this more general, and following Kotlin best practices.

Answer 1

1. You don't need a wasPresent variable. MutableSet.add returns true if the element was added to the set:

   if (observers.add(observer)) {
       init(observer)
   }
   

2. Sometimes in Kotlin you don't have to declare temporary variables so that you can do something with a return value before returning it. e.g.:

   return ObservedProperty(initialValue, onChanged).apply { properties.add(this) }
   

   Unfortunately this can sometimes look a bit strange using this, etc. but in Kotlin 1.1 you can use bound callable references:

   return ObservedProperty(initialValue, onChanged).apply(properties::add)
   

3. You might want to take a look at Delegates.observable - stdlib - Kotlin Programming Language which you may be able to reuse to improve and reduce your code.

Answer 2

I'm going to post a slightly different opinion; the previous answer is perfectly sensible but they aren't the changes I'd make.

1. I'd encourage you to use the += operator instead of add. The reason being it gives you a kind of duck-typing with collections: your code (at a source compatibility layer) stops caring about the specific implementation of the add like thing, and instead asks if the type implements a fairly common operator. The single most obvious advantage here is that it allows you to switch from val mutableCollection to var immutableCollection completely without (source-compatability) issue.

   this also negates your ability to use add for its return value, since operator += is defined to return Unit (void), so you'll need to formalize your membership info (wasPresent) in a side channel.

2. I don't know how much advantage there is to a unified 2-method PointObserver interface, at least as anything other than an implementation detail. You might consider dropping that interface in favor of onXChanged and onYChanged (and possibly onXOrYChanged).

3. You might also use a sealed class to implement your point observer, rather than an interface.

combining suggestions 2 & 3:

class Point {
  val observers: ObserverSet<PointObserver>()
  val x by observers.observe<XChanged>(0f)
  val y by observers.observe<YChanged>(0f)

  fun onXChanged(handler: (old: Float, new: Float) -> Unit) = observers += XChanged(handler)
  fun onYChanged(handler: (old: Float, new: Float) -> Unit) = observers += YChanged(handler)
}

sealed class PointObserver()
class XChanged(val handler: (Float, Float) -> Unit): PointObserver()
class YChanged(val handler: (Float, Float) -> Unit): PointObserver()

class PointView(point: Point) {
  init {
    point.onXChanged { old, new -> ... }
    point.onYChanged { old, new -> ... }
  }
}

which would require an implementation change:

fun <reified T: PointObserver> ObservableSet<PointObserver>.observe(default: Float)
    = ObservedProperty(initialValue) { it is T }

private inner class ObservedProperty<V>(private var value: V, private val handlerFilter: (PointObserver) -> Boolean) :
        ReadWriteProperty<Any?, V> {

    override fun getValue(thisRef: Any?, property: KProperty<*>) = value

    override fun setValue(thisRef: Any?, property: KProperty<*>, newValue: V) {
        val oldValue = this.value
        this.value = newValue
        observers.filter(handlerFilter).forEach { observer ->
            when(observer){
               is XChanged -> observer.handler(oldValue, newValue)
               is YChanged -> observer.handler(oldValue, newValue)
            }
        }
    }

    fun initObserver(observer: O) {
        observer.onChanged(value, value)
    }
}

---

Note however that this should really only be used for instructional purposes, there are plenty of libraries out there that have built-in observer pattern implementations, not the least of which is guava. You're likely much better off using one of those libraries than suffering from the inner-platform effect.