Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Listener types #2

Open
hendrikebbers opened this issue Jan 21, 2017 · 8 comments
Open

Listener types #2

hendrikebbers opened this issue Jan 21, 2017 · 8 comments

Comments

@hendrikebbers
Copy link
Member

Some ideas based on the API of Swift (https://developer.apple.com/library/content/documentation/Swift/Conceptual/Swift_Programming_Language/Properties.html#//apple_ref/doc/uid/TP40014097-CH14-ID254). Here they provide a willSet(...) and didSet(...) while in the willSet(...) you can access newValue, in didSet(...) it is simply called value. Maybe we should think about something like a willSet(...) listener that can be used for validation etc. Based on some discussions I would currently prefer:

  • 1 listener type that will be called before the change. This listener will be called always (even if new value is equals old value). This listeners needs to have access to the old and new values. Maybe this can return a boolean value to interrupt the change based on validation etc.
  • 1 listener type that will be called after the change. This listener will only be called if the value really changed (old != new). Here you can access the value and the old value.
@HanSolo
Copy link

HanSolo commented Jan 22, 2017

I like that idea, esp. that there is a listener which will always be fired, even if the value did not change.

@hastebrot
Copy link

hastebrot commented Jan 31, 2017

Again, my idea is to keep uniformity and have something like before().onChange() instead of onWillChange(). The problem with this idea is, that we need probably can't chain them like onWillChange(it => ...).onChange(it => ...).

For reference, a motivating example is in section "Property Observers":

class StepCounter {
    var totalSteps: Int = 0 {
        willSet(newTotalSteps) {
            print("About to set totalSteps to \(newTotalSteps)")
        }
        didSet {
            if totalSteps > oldValue  {
                print("Added \(totalSteps - oldValue) steps")
            }
        }
    }
}

My first remark is, that willSet implies we have something like getValue() in Observable, i.e. the Observable (and Property) has a "current" value that can change over time, and we can compare it to the previous value.

Events as comparison don't have this concept (see illustration below; courtesy of Kefir [1]). Note, that it is easy to convert from event streams to a property with something like buffer(2) window(2, 1). Also note, that with reactive streams the whole concept of mutable values vanishes. While immutability is a good property, we might want something like getValue() and setValue() and ReactFX has shown, that we can use (mutable) properties as base and later wrap them (probably with an external library) into (immutable) event streams.

stream-and-property

[1] https://rpominov.github.io/kefir/#about-observables

Compared to JavaFX there are two things:

  • javax.observer.Observable has getValue() and is somewhat a combination of javafx.beans.Observable and javafx.beans.value.ObservableValue.
  • javax.observer.Property has setValue() and is something like javafx.beans.value.WritableValue.

@hastebrot
Copy link

hastebrot commented Jan 31, 2017

I also thought about avoiding the whole functional rabbit hole and make use of OOP polymorphism with different listener types.

onChange(new ChangeListener() { ... })
onChange(new WillChangeListener() { ... })

This would move a lot of code logic from the observables into the listeners.

Update: I see, there is also a veto() method. Maybe (code in Groovy for simplicity):

onChange({ it > 10 ? approve(it) : reject() } as WillChangeListener)

This will change the passive nature of the listeners to a more active one (whether good or not?).

@hendrikebbers
Copy link
Member Author

Feature request that wants access to the Subscriptions of a Property. Does this sounds right? Is such an access needed?
https://github.com/canoo/dolphin-platform/issues/44

@hendrikebbers
Copy link
Member Author

@hastebrot

onChange(new ChangeListener() { ... })
onChange(new WillChangeListener() { ... })

I do not like to name both methods the same. By doing so you always need to "cast" the lambda expression and this ends in more code than by 2 simple different methods name.

In addition "onChange" is wrong for both methods (one happens before the change and one after the change. No method will be called at the change)

@hendrikebbers
Copy link
Member Author

@hastebrot

onChange({ it > 10 ? approve(it) : reject() } as WillChangeListener)

Interesting. By doing so you could manipulate the value (another param is passed to approve(...)). I do not know if this is a good functionality or confusing.

It will be good if you have a max String length and someone copies a large string. In that case you can approve a substring. But: What happens if a listener do not call any of the 2 methods? Is this like v -> approve(v) or will it work like v -> reject() or will this end in an exception?

@hendrikebbers
Copy link
Member Author

@hastebrot

Again, my idea is to keep uniformity and have something like before().onChange() instead of onWillChange().

What is the return type of before() in this case?

@hendrikebbers
Copy link
Member Author

As a starting point I will merge the current PR. Based on this we can continue the discussion

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants