Skip to content
This repository has been archived by the owner on Mar 16, 2021. It is now read-only.

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

Merged
merged 2 commits into from
Feb 21, 2018

Conversation

rehlma
Copy link
Contributor

@rehlma rehlma commented Feb 20, 2018

No description provided.

@StefMa
Copy link
Contributor

StefMa commented Feb 20, 2018 via email

@jreehuis
Copy link

@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
Copy link
Contributor

passsy commented Feb 20, 2018

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

@StefMa
Copy link
Contributor

StefMa commented Feb 20, 2018

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
Copy link
Contributor

StefMa commented Feb 20, 2018

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
Copy link

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

@passsy
Copy link
Contributor

passsy commented Feb 20, 2018

@RestrictTo(SUBCLASSES) sound great!

@rehlma
Copy link
Contributor Author

rehlma commented Feb 21, 2018

Great! Just did it.

@passsy passsy merged commit 0544bb2 into master Feb 21, 2018
@passsy passsy deleted the prepare_for_extension branch February 21, 2018 16:47
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

Successfully merging this pull request may close these issues.

4 participants