Skip to content
This repository has been archived by the owner on Oct 25, 2024. It is now read-only.

Give presenters a safer hook for view detaching #156

Open
rjrjr opened this issue May 4, 2015 · 12 comments
Open

Give presenters a safer hook for view detaching #156

rjrjr opened this issue May 4, 2015 · 12 comments

Comments

@rjrjr
Copy link
Collaborator

rjrjr commented May 4, 2015

Drop getView(), replace onLoad(Bundle) with onTakeView(View,Bundle) and onDropView(), make dropView() final.

Lifecycle becomes:

enterScope()
  onTakeView(View, Bundle)
  onDropView()
exitScope()

With onSave(Bundle) being called any arbitrary number of times between enter and exit.

Also worth thinking about decoupling Presenter and Bundle entirely.

@loganj
Copy link
Collaborator

loganj commented May 4, 2015

I feel pretty good about decoupling Presenter and Bundler. That should be a pretty mechanical change to delegation, and would significantly simplify Presenter.

Less sure about de-scoping Bundler.

@rjrjr
Copy link
Collaborator Author

rjrjr commented May 4, 2015

Paves the way to closing issue #103

@pyricau
Copy link
Member

pyricau commented May 4, 2015

Just to clarify: does that mean that we'll need to reimplement the current Presenter view handling code in a subclass?

@rjrjr
Copy link
Collaborator Author

rjrjr commented May 5, 2015

No. It means the contract would be that you can hold on to the View passed into onTakeView() yourself, and you should drop it when onDropView() is called. But the current debouncing behavior would still be in place.

We just no longer have a getView() method that's hard to document. Instead there's "here's your view," and "you should forget about that view now".

On Mon, May 4, 2015 at 4:44 PM Pierre-Yves Ricau [email protected]
wrote:

Just to clarify: does that mean that we'll need to reimplement the current
Presenter view handling code in a subclass?


Reply to this email directly or view it on GitHub
#156 (comment).

@pyricau
Copy link
Member

pyricau commented May 5, 2015

Ok. So technically the change means:

  • Add a view field in all the presenters
  • assign that field in takeView
  • Implement onDropView in all presenters and clear the reference
  • replace all hasView() calls with view != null

@loganj
Copy link
Collaborator

loganj commented May 5, 2015

My bet:

  1. Most presenters don't even need to keep a field reference to the view.
  2. This change makes presenters a lot easier to understand because the template methods are actually symmetric and there's no superclass magically managing a field that you have to assume might be null at any moment.

If my bet is wrong for your particular app, there's nothing stopping you from introducing a Presenter subclass to use as a base class for your own presenters.

@rjrjr
Copy link
Collaborator Author

rjrjr commented May 5, 2015

The change to make the API symmetric is separate from the decision to hide
getView(), so your second point doesn't bolster your argument. That
leaves us with typical use, and I think the jury is still out on that.

I do like the fact that the new API is more minimal, less side-effecty.

@loganj
Copy link
Collaborator

loganj commented May 5, 2015

I don't think it's a completely separate decision. Making the API symmetric makes getView/hasView superfluous.

@rjrjr
Copy link
Collaborator Author

rjrjr commented May 12, 2015

Talked about this some more. Also starting to seem like enterScope / exitScope should not be built into presenter. Orthogonal concerns.

@urandom
Copy link

urandom commented Nov 8, 2015

Would it be possible to somehow obtain the bundle during onDropView(), so that extra stuff about the view can be saved and later easily obtained back during onTakeView()?

@valeriyo
Copy link
Contributor

valeriyo commented Dec 8, 2015

Good stuff. I assume onTakeView / onDropView could be repeated N times (e.g. on rotation) ?

Btw, whenever view invokes presenter's callbacks, it could just pass this as parameter - no need for getView() in that case.

@rjrjr
Copy link
Collaborator Author

rjrjr commented Dec 9, 2015

Yes, exactly.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants