#598 Unified notification API

qualidafial Fri 15 May 2009

Breaking out this topic from #589:

At some point in the future it would be beneficial to add bound fields to Fan with support for change notifications baked into the language. Although this is not strictly necessary for 1.0, there are existing notification APIs in fwt that may become obsolete or redundant in the future.

What I propose is for the language to standardize now (i.e. before Fan 1.0 release) on a few core classes so that APIs will remain consistent later on:

class Event {
  Obj source
  Obj id // event id
}

class Listeners {
  Void add(|Event| cb)
  Void remove(|Event| cb)
  Void send(Event event)
}

mixin Notify {
  Listeners onEvent(Obj id)
  Void notify(Event event) {
    onEvent(event.id).send(event)
  }
}

An example class adopting the Notify API as described above:

class Person : Notify {
  String name {
    set {
      @name = val
      notify( Event {
        source = this
        id = #name
      } )
    }
  }

  private Obj:Listeners listeners

  Listeners onEvent(Obj id) {
    return listeners[id] ?: listeners[id] = Listeners()
  }
}

One aspect I'm still going back and forth on is how to support listening to any event on the source object, instead of having to add a listener for every individual event id. I've considered two options:

mixin Notify {
  Listeners onEvent(Obj id)
  Listeners onAny()
  Void notify(Event event) {
    onEvent(event.id).send(event)
    onAny.send(event)
  }
}

With this structure you end up with this boilerplate in every implementer:

private Obj:Listeners listeners

Listeners onEvent(Obj id) {
  return listeners[id] ?: listeners[id] = Listeners()
}

once Listeners onAny() { return Listeners() }

This is more boilerplate than I like, and you end up with a separate Listeners instance for every type of event supported plus one for the any listeners. Another option:

mixin Notify {
  Listeners listeners()
}

class Listeners {
  Void add(Obj id, |Event| cb)
  Void remove(Obj id, |Event| cb)

  Void addAny(|Event| cb)
  Void removeAny(|Event| cb)

  Void notify(Event event)
}

Personally I'm leaning toward the second option since you end up a single, keyed list at runtime and run less risk of dangling object references. I like having the Listeners object be completely responsible for managing listeners so this doesn't have to be punted to the implementor. Another variant on the above:

class Listeners {
  private Obj:ListenerList eventListeners := [:]

  ListenerList onEvent(Obj id) {
    return eventListeners[id] ?: eventListeners[id] = Listeners()
  }

  private ListenerList? anyListeners := null

  Listeners onAny() {
    return anyListeners ?: anyListeners = Listeners()
  }

  Void notify(Event event) {
    listener[event.id]?.send(event)
    anyListeners?.send(event)
  }
}

class ListenerList {
  Void add(|Event| cb)
  Void remove(|Event| cb)
  Void send(Event event)
}

This is similar to the first proposal but offloads the default implementation (and thus most of the boilerplate) into Listeners, so all an implementor has to do is:

once Listeners listeners() { return Listeners() }

Looking forward to everyone's feedback.

qualidafial Fri 15 May 2009

Some example code for using the API according to the last option above:

Person person = Person { name = "Bob" }
person.listeners.onEvent(Person#name).add |Event e| {
  echo("name changed")
}
person.name = "Tom" => "name changed"

I don't like the extra long chain, so maybe add a few convenience methods to the Notify mixin:

mixin Notify {
  Listeners listeners()
  ListenerList onEvent(Obj id) { return listeners.onEvent(id) }
  ListenerList onAny() { return listeners.onAny }
  Void notify(Event e) { listeners.notify(e) }
}

class Person : Notify {
  String name {
    set {
      @name = val
      notify( Event {
        source = this
        id = #name
      } )
    }
  }

  once Listeners listeners() { return Listeners() }
}

Then the usage example above becomes:

Person person = Person { name = "Bob" }
person.onEvent(Person#name).add |Event e| {
  echo("name changed")
}
person.name = "Tom" => "name changed"

JohnDG Fri 15 May 2009

I personally do subscriptions based on the class of the Event (type, in the case of Fan). This removes the need for any notion of id and is generally the best approach, since a listener can only deal with events whose class it knows.

Then a client can listen to any event by subscribing to events of type Obj#.

qualidafial Fri 15 May 2009

@JohnDG: If we did it this way, you'd end up with a new event class for every distinct property:

class Person : Notify
{
  String firstName {
    set
    {
      @firstName = val
      notify(PersonFirstNameChangeEvent.make)
    }
  }

  String lastName {
    set
    {
      @lastName = val
      notify(PersonLastNameChangeEvent.make)
    }
  }
}

class PersonFirstNameChangeEvent : Event { }
class PersonLastNameChangeEvent : Event { }

etc.

brian Sun 17 May 2009

Originally I thought you were talking about adding language support for auto-magically compiling property change events.

But this proposal seems more focused on pulling the reusable pieces of fwt Event and EventListerns down into sys to make them more reusable. That seems quite reasonable.

I favor keeping the model they way it is being used by fwt, an EventListerners per event. It seems a lot cleaner and easier to understand. You can push the complexity into the Event subclasses themselves, or add helper methods to your class for batch adding of a listener. For example, I like a single onChange event on my components and the event stores which property was add, removed, or modified.

If I look at what we would do in though, I wouldn't want a Event class in sys, rather I'd probably just use Obj. So the only class we'd really move would be EventListeners. And I think whether we did that now or in the future it really wouldn't make a big difference (it is a pretty light weight class).

But I suspect, Andy and I might be up for tackling the larger data binding problem in the near future (of which this is a part).

tompalmer Mon 18 May 2009

But I suspect, Andy and I might be up for tackling the larger data binding problem in the near future (of which this is a part).

That's cool. Data binding is sweetness. I just haven't thought enough about it to have a serious comment on it myself so far.

qualidafial Mon 18 May 2009

Originally I thought you were talking about adding language support for auto-magically compiling property change events.

That is what I was originally proposed. Your responses to the original proposal suggested that there wasn't enough time for full property change support in 1.0--this proposal is to lay down the core elements so that in the future, when data binding is eventually supported, there will be one unified notification mechanism instead of multiple mechanisms to choose from depending on the library.

qualidafial Mon 18 May 2009

I favor keeping the model they way it is being used by fwt, an EventListerners per event.

Just a side-note, shouldn't each of these EventListeners fields be once methods instead? That way you get lazy initialization.

I like a single onChange event on my components and the event stores which property was add, removed, or modified.

This is a nice, clean approach, but unfortunately it pushes the check for event id into every listener. That is, instead of:

Person p := ...
p.onChange(Person#firstName).add |ChangeEvent e|
{
  doSomething()
}

you get:

Person p := ...
p.onChange.add |ChangeEvent e|
{
  if (e.id == Person#firstName)
  {
    doSomething()
  }
}

which is a lot of boilerplate to push into most listeners.

If I look at what we would do in though, I wouldn't want a Event class in sys, rather I'd probably just use Obj. So the only class we'd really move would be EventListeners. And I think whether we did that now or in the future it really wouldn't make a big difference (it is a pretty light weight class).

One thing that occurred to me is that unless EventListeners becomes a generified class, this will force every listener to cast the event argument to the correct subclass:

p.onChange.add |Obj o|
{
  ChangeEvent event := o
  doSomething()
}

Suggesting that maybe EventListeners should perhaps be replaced by checked lists.

brian Tue 19 May 2009

Just a side-note, shouldn't each of these EventListeners fields be once methods instead? That way you get lazy initialization.

Actually that is a good idea, since they never get written to again.

One thing that occurred to me is that unless EventListeners becomes a generified class, this will force every listener to cast the event argument to the correct subclass

It won't be typed per se, but you could still write your callbacks like:

p.onChange.add |ChangeEvent e|
{
  doSomething()
}

However, you would loose the ability to use type inference on your closure (which kind of sucks).

tompalmer Tue 19 May 2009

It won't be typed per se, but you could still write your callbacks like

Which is part of why I still promote support for arbitrary co/contravariance for all parameter and return types at the method/slot level. But that's a loosening of existing rules (and wouldn't change signatures behind the scenes at the JVM/.NET level -- since they'd stick behind the scenes to whatever the overridden method required) and could be added after 1.0.

I just saw the chance to keep it on the radar, and that's all I'm doing for now.

Login or Signup to reply.