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:
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:
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:
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.
qualidafialFri 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"
JohnDGFri 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#.
qualidafialFri 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.
brianSun 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).
tompalmerMon 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.
qualidafialMon 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.
qualidafialMon 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.
brianTue 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).
tompalmerTue 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.
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:
An example class adopting the Notify API as described above:
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:
With this structure you end up with this boilerplate in every implementer:
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 theany
listeners. Another option: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:
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:
Looking forward to everyone's feedback.
qualidafial Fri 15 May 2009
Some example code for using the API according to the last option above:
I don't like the extra long chain, so maybe add a few convenience methods to the
Notify
mixin:Then the usage example above becomes:
JohnDG Fri 15 May 2009
I personally do subscriptions based on the
class
of theEvent
(type
, in the case of Fan). This removes the need for any notion ofid
and is generally the best approach, since a listener can only deal with events whoseclass
it knows.Then a client can listen to
any
event by subscribing to events of typeObj#
.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:
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
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
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
Just a side-note, shouldn't each of these
EventListeners
fields beonce
methods instead? That way you get lazy initialization.This is a nice, clean approach, but unfortunately it pushes the check for event id into every listener. That is, instead of:
you get:
which is a lot of boilerplate to push into most listeners.
One thing that occurred to me is that unless
EventListeners
becomes a generified class, this will force every listener to cast theevent
argument to the correct subclass:Suggesting that maybe EventListeners should perhaps be replaced by checked lists.
brian Tue 19 May 2009
Actually that is a good idea, since they never get written to again.
It won't be typed per se, but you could still write your callbacks like:
However, you would loose the ability to use type inference on your closure (which kind of sucks).
tompalmer Tue 19 May 2009
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.