sockeqwe / mosby

A Model-View-Presenter / Model-View-Intent library for modern Android apps
http://hannesdorfmann.com/mosby/
Apache License 2.0
5.49k stars 841 forks source link

Consider not removing getView() on next releases #282

Closed KushtrimPacaj closed 6 years ago

KushtrimPacaj commented 6 years ago

Hi, I see that the getView() method on MvpBasePresenter is deprecated in v3.1.0 , in favor of #ifViewAttached(ViewAction) https://github.com/sockeqwe/mosby/blob/de117810d05b51dd31b743daf472dc94c9cf8d82/mvp/src/main/java/com/hannesdorfmann/mosby3/mvp/MvpBasePresenter.java#L86

While I agree that the new syntax is nice for Java, I think that for Kotlin the old syntax works fine in most cases, thanks to its safe call operator and property access syntax ( See the example below )

mosby
sockeqwe commented 6 years ago

It has other reasons why this shouldn't be used in java (it's not about nicer syntax). Mainly because of garbage collection and WeakReference, see https://github.com/sockeqwe/mosby/issues/233

KushtrimPacaj commented 6 years ago

Ok, that makes sense. But the problem doesn't carry on the view?. one line use case.

The getView would either get a strong reference on which we call the method, or null in which case nothing happens.

Anyways, the suggestion was just not to remove it from the library, it may be kept deprecated in favor of the more foolproof API

sockeqwe commented 6 years ago

It will be kept deprecated to not break backward compatibility.

Although you dont have that problem described in #233 by using kotlin per se but you still can run (theoretically, in practice not very likely to happen) in this "issue":

view?.setData( someData) // This runs fine because view != null
// <-- gc happens -->
view?.hideProgressBar() // view == null --> never executed 

It's not really an issue for the most apps, but perhaps in some edgecases it is important to run either both view.setData() and view.hideProgressbar() or none of them. ifViewAttached() guarantees that there is a view != null within the lambda or doesn't run at all:

ifViewAttached { view ->
    view.showData( someData )
    view.hideProressBar()
}