guigarage / ObserverPattern

Basic Java observer API
Apache License 2.0
7 stars 7 forks source link

Listener types #2

Open hendrikebbers opened 7 years ago

hendrikebbers commented 7 years ago

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:

HanSolo commented 7 years ago

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

hastebrot commented 7 years ago

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:

hastebrot commented 7 years ago

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 commented 7 years ago

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 commented 7 years ago

@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 commented 7 years ago

@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 commented 7 years ago

@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 commented 7 years ago

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