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

[Proposal] - Make FeedbackLoop as a struct #11

Open
sergdort opened this issue Oct 14, 2017 · 9 comments
Open

[Proposal] - Make FeedbackLoop as a struct #11

sergdort opened this issue Oct 14, 2017 · 9 comments

Comments

@sergdort
Copy link

Hi, guys!

I've been thinking how we can improve API in a way that users won't be able to path their own FeedbackLoop as they may use it wrong by not using scheduler correctly

So here is my proposal:

public struct FeedbackLoop<State, Event> {
    let loop: (ImmediateSchedulerType, Observable<State>) -> Observable<Event>
    
    public init<Control: Equatable>(query: @escaping (State) -> Control?, effects: @escaping (Control) -> Observable<Event>) {
        self.loop = { scheduler, state -> Observable<Event> in
            return state.map(query)
                .distinctUntilChanged { $0 == $1 }
                .flatMapLatest { control -> Observable<Event> in
                    guard let control = control else { return Observable.empty() }
                    
                    return effects(control).enqueue(scheduler)
                }
        }
    }
    
    public init(predicate: @escaping (State) -> Bool, effects: @escaping (State) -> Observable<Event>) {
        self.loop = { scheduler, state -> Observable<Event> in
            return state.flatMapLatest { state -> Observable<Event> in
                guard predicate(state) else { return Observable.empty() }
                
                return effects(state).enqueue(scheduler)
            }
        }
    }
    
    public init(effects: @escaping (State) -> Observable<Event>) {
        self.loop = { scheduler, state in
            return state.flatMapLatest { state in
                return effects(state).enqueue(scheduler)
            }
        }
    }
}

By exposing only these three available initializers. I guess we can also somehow figure out how something similar can be adopted for Driver as well.

What do you guys think?

@kzaher
Copy link
Member

kzaher commented Oct 15, 2017

Hi @sergdort ,

How would we extend it? It seems to me that only way would be to expose loop lambda, but if we do that, then it's still possible to mess up.

I guess this would also maybe reduce chances of error.

public enum FeedbackLoop<State, Event> {
    case startRequest(query: (State) -> Control?, effects: (Control) -> Observable<Event>)
    ...
    case custom((ImmediateSchedulerType, Observable<State>) -> Observable<Event>)
}

extension FeedbackLoop {
     var loop: (ImmediateSchedulerType, Observable<State>) -> Observable<Event>
}

That would allow us to give names to most common feedback loops.

@sergdort
Copy link
Author

It seems to me that only way would be to expose loop lambda, but if we do that, then it's still possible to mess up.

I'm a little bit confused :) The idea is to not expose loop closure but only expose initializers

@kzaher
Copy link
Member

kzaher commented Oct 16, 2017

@sergdort yeah, I understand that, but how would somebody create an arbitrary feedback loop if needed? Don't think there is a finite number of them.

@sergdort
Copy link
Author

sergdort commented Oct 16, 2017

Do you think this one is not enough? Or you think someone would want to use something else then flatMapLatest ?

public init(effects: @escaping (State) -> Observable<Event>) {
        self.loop = { scheduler, state in
            return state.flatMapLatest { state in
                return effects(state).enqueue(scheduler)
            }
        }
    }

@kzaher
Copy link
Member

kzaher commented Oct 16, 2017

I don't think it's just a matter of customizing flatMapLatest.

E.g. there is this form that uses a set that controls it:

public func react<State, Control: Hashable, Event>(
    query: @escaping (State) -> Set<Control>,
    effects: @escaping (Control) -> Driver<Event>
    ) -> (Driver<State>) -> Driver<Event> {
    return { state in
        let query = state.map(query)

        let newQueries = Driver.zip(query, query.startWith(Set())) { $0.subtracting($1) }

        return newQueries.flatMap { controls in
            return Driver.merge(controls.map { control -> Driver<Event> in
                return query.filter { !$0.contains(control) }
                    .map { _ in Driver<Event>.empty() }
                    .startWith(effects(control).enqueue())
                    .switchLatest()
            })
        }
    }
}

I'm assuming one could potentially want concat in case events represents some form of commands that mutate state and can't be cancelled vs queries.

It's hard to anticipate all of the possible combinations :), I'm not sure we should assume there is a finite number of them.

@sergdort
Copy link
Author

Oh, I see :)

Anyway, isn't the difference only in "flatting" strategy? Can't think of anything else.

@sergdort
Copy link
Author

Ignore me =) I just read code snipped above one more time...

@kzaher
Copy link
Member

kzaher commented Oct 16, 2017

Actually, the flattening strategy here is the same ;)

@kzaher
Copy link
Member

kzaher commented Oct 16, 2017

I guess we can probably improve them in some way ...

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

2 participants