Skip to content
This repository has been archived by the owner on May 23, 2020. It is now read-only.

Callbacks before and after super call #36

Closed
passsy opened this issue Jan 25, 2016 · 14 comments
Closed

Callbacks before and after super call #36

passsy opened this issue Jan 25, 2016 · 14 comments

Comments

@passsy
Copy link
Contributor

passsy commented Jan 25, 2016

Libraries like Flow execute code before the super call. This is currently not supported by navi.

@dlew
Copy link
Contributor

dlew commented Jan 25, 2016

Do you have a proposal for how this would be supported?

@passsy
Copy link
Contributor Author

passsy commented Jan 26, 2016

The easiest solution would look like this. Two events for every lifecycle method, one before and one (the normal) after the super call.

NaviActivity

  @Override protected void onResume() {
    base.onResumeBeforeSuper()
    super.onResume();
    base.onResume();
  }

NaviEmitter

  public void onResumeBeforeSuper() {
    emitEvent(Event.RESUME_BEFORE_SUPER);
  }

Possible problems

The ALL listener would run into problems. It's not possible to distinguish between the before and after events. //cc #30

@dlew
Copy link
Contributor

dlew commented Jan 26, 2016

That's the easy part, I meant more about how to modify the interface such that it doesn't make things a lot more complicated.

@passsy
Copy link
Contributor Author

passsy commented Jan 26, 2016

Not sure if I got this question correct.
The interface (this?) doesn't have to be tweaked for normal events. I don't see why more events make this lib more complicated. Simply register for whatever you need.

But the ALL listener requires this interface as proposed in #30.

public interface AllListener<T> {
  void call(Event<T> event, T data);
}

@dlew
Copy link
Contributor

dlew commented Jan 26, 2016

Oh, so you mean a whole second set of events for before. Hmm... I'll have to think about it.

@dlew
Copy link
Contributor

dlew commented Jan 28, 2016

An alternative solution would be to allow you to specify it when adding the event, aka:

<T> void addListener(Event<T> event, Listener<T> listener, boolean beforeSuper);

Then the event list would remain the same.

Adding a new parameter would make the API slightly more complex but maybe it's worth it for this flexibility.

@passsy
Copy link
Contributor Author

passsy commented Jan 28, 2016

All events I can think of have a super call. So, the optional beforeSuper(default: false) parameter is fine. Do you think there could be any kind of event where it doesn't make sense to use beforeSuper: false?

@dlew
Copy link
Contributor

dlew commented Feb 1, 2016

The more I think about this, the more I like it. Right now we're actually mixing when we call super - in most cases it's after, but in cases where a value is returned (e.g. CREATE_VIEW) we call it before. That's confusing and that knowledge shouldn't be required by consumers.

I think requiring the specification of before/after super will make the API more flexible and more clear, since you don't have to wonder when it's being called in the lifecycle.

@cypressious
Copy link

Any news on this?

@dlew
Copy link
Contributor

dlew commented Feb 17, 2016

When I have spare time I'll be playing around with this more. I've got it half-baked over here.

@cypressious
Copy link

In my experience, all of the work in the destruction callbacks is usually done before the super call while the work in the creation callbacks is usually done after the super call. This is backed at least by this SO answer: http://stackoverflow.com/a/9626268/615306. I think Navi should do the same, no need for extra parameters and duplicate events.

@dlew
Copy link
Contributor

dlew commented Feb 27, 2016

Yeah, I think that's reasonable, as long as it's better documented.

The more I played around with the extra parameter, the more I got annoyed with it. It's just a lot of hassle for a corner case.

@passsy
Copy link
Contributor Author

passsy commented Feb 27, 2016

Calling the initializer super methods first and the finalizer methods after super is generally true. But some cases require doing something special, i.e. setTheme() normally gets called before super.onCreate(). And as @commonsguy said, it's up to the implementation logic when to call super.onActivityResult().

I'm really hoping navi will be general enough to get callbacks for both.

@dlew
Copy link
Contributor

dlew commented Feb 27, 2016

I thought you just needed to call setTheme() before setContentView(), not super.onCreate().

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

3 participants