GCX-HCI / ThirtyInch

a MVP library for Android favoring a stateful Presenter
Apache License 2.0
1.03k stars 101 forks source link

Made sendToView public to allow to override or extend the function. #142

Closed rehlma closed 6 years ago

StefMa commented 6 years ago

You could extend or override the function with the protection modifier as well.

What is your use case? 🤔

On Feb 20, 2018 2:58 PM, "rehlma1989" notifications@github.com wrote:


You can view, comment on, or merge this pull request online at:

https://github.com/grandcentrix/ThirtyInch/pull/142 Commit Summary

  • Made sendToView public to allow to override or extend the function.

File Changes

Patch Links:

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/grandcentrix/ThirtyInch/pull/142, or mute the thread https://github.com/notifications/unsubscribe-auth/AJwYe6zlk6pftkldf74I0X_8NLG5zIQIks5tWs-ZgaJpZM4SL_l8 .

jreehuis commented 6 years ago

@rehlma1989 This method is intended just for Presenter internal use. So it should not be opened to external classes. And as @StefMa said, in the actual Presenter it is overridable anyways.

passsy commented 6 years ago

The protection modifier prevents us to create an extension function in Kotlin which uses sendToView

StefMa commented 6 years ago

To clean up some things: The goal idea was to create a kotlin extension like:

fun <V : TiView> TiPresenter<V>.sendToViewKotiln(block: (V.() -> Unit)) {
    sendToView { v -> v.block() }
}

This will give us the benefit that the receiver is not a param (it) but this. Which means instead of having this:

fun aMethod() {
   sendToView { it.showLoading(); it.showAnotherThings(); it.doWhatever() }
}

we can use:

fun aMethod() {
   sendToView { showLoading(); showAnotherThings(); doWhatever() }
}

Since Kotlin extensions are just static java methods we can't create such a extension without a public method. sendToView is protected and we haven't access to it from a static method. That is the reason why this PR was opened...

StefMa commented 6 years ago

To speak about pros/cons:

After seeing the benefits of it I'm not against this change. Normally the presenter is only available/accessable via a View (Activtiy/Fragment). And I think no one will ever use presenter.sendToView {} from a View...

jreehuis commented 6 years ago

That UseCase definitely make sense. Is it possible to restrict the usage than by @RestrictTo(RestrictTo.Scope.SUBCLASSES) or another scope?

passsy commented 6 years ago

@RestrictTo(SUBCLASSES) sound great!

rehlma commented 6 years ago

Great! Just did it.