Skip to content

Conversation

@sdelaysam
Copy link
Contributor

@dmdevgo I'm considering your great framework to use in the next projects.
One thing that kinda bothers me is the need to write this code again and again

state.consumer.accept(...)
//or
observable.subscribe(state.consumer)

I understand this meant to unify the API but for end users it may be nice to have some convenience over it.

@dmdevgo
Copy link
Owner

dmdevgo commented Oct 3, 2019

@sdelaysam Thank you for the suggested improvements. The convenience issue is more complicated than it seems. For example, there is a need to write boilerplate code again or again like this .subscribe().untilDestroy() or correctly subscribe to the Action. Please see #46 and give us feedback on the proposed solution.

@sdelaysam
Copy link
Contributor Author

sdelaysam commented Oct 3, 2019

@dmdevgo for me #46 decreased code readability. I like regular code .subscribe().untilXXX() (even if its repetitive) way more than Rx code in lamdas or initializers.
On the other hand, its fine for me to collapse several Rx operators into one if possible (like you did with bindTo)
Probably thats the way to go? (I'm not suggesting to name it "bindTo", just to illustrate the idea)

val count = state(initialValue = 0)
val buttonEnabled = state(false)

fun onCreate() {
    count.map { it > 0 }.bindTo(buttonEnabled)
}

Copy link
Collaborator

@Jeevuz Jeevuz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sdelaysam Thanks for your proposals. We've discussed with @dmdevgo and I'm reviewed your changes and leaved comments with changes we ask you to make. Thanks in advance!

@sdelaysam
Copy link
Contributor Author

@Jeevuz done!

@dmdevgo
Copy link
Owner

dmdevgo commented Nov 1, 2019

@sdelaysam resolve conflicts, please

@Jeevuz Jeevuz self-requested a review November 2, 2019 10:03
# Conflicts:
#	sample/src/main/kotlin/me/dmdev/rxpm/sample/counter/CounterPm.kt
@sdelaysam
Copy link
Contributor Author

@dmdevgo done!

@Jeevuz Jeevuz merged commit c2908e9 into dmdevgo:develop Nov 2, 2019
@Jeevuz
Copy link
Collaborator

Jeevuz commented Nov 2, 2019

@sdelaysam Thanks!

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

Successfully merging this pull request may close these issues.

3 participants