Skip to content
This repository has been archived by the owner on Feb 26, 2023. It is now read-only.

Extend new Fragment Scope for EBean #2214

Open
smaugho opened this issue Feb 17, 2019 · 10 comments
Open

Extend new Fragment Scope for EBean #2214

smaugho opened this issue Feb 17, 2019 · 10 comments

Comments

@smaugho
Copy link
Contributor

smaugho commented Feb 17, 2019

We resently added Fragment Scope for EBean annotated classes on #2206. Its usage right now is simply to ensure that the EBean is the same if it is injected in the same fragment scope. There is some cases of uses in which could be useful, though there are other cases in which its usability increases.

It is common to have fragments which contains multiple subfragments. Or to have a collection of fragments which are related in functionality.

I do propose to extend the Fragment scope for this cases, for doing so, @EBean should support 2 extra parameters, fragmentId, and fragmentTag. This is an example of usage:

@EBean(scope = Scope.Fragment, fragmentId = {R.id.fragment1, R.id.fragment2})
public class SomeBean {
    //Code
}

This will increase the usability of the new Fragment scope. You could have complete sections on the app sharing the same instance of a Bean, which is really useful.

For doing this, basically to inject the Bean, it is used exactly the same mechanism implemented on #2206, merged with a search of Fragment by ID or by Tag (similar to how the @FragmentById or @FragmentByTag work).

The fragment will be injected using the first BeanHolder found, so if a serie of fragmentsId is provided, the first one would be the "holder" of the bean. In the case of not existing the second and so on.

In cases where there is a "parent fragment" of course, the best is that the programmer select this parent fragment as the holder (only providing its ID to the @EBean) (like a fragment with a ViewPager which contains multiple other fragments).

If the parameters fragmentId or fragmentTag are not provided, then it works exactly how it is currently implemented.

@WonderCsabo
Copy link
Member

I think it would overload the @EBean annotation. Maybe we should add another annotation instead, but i still think this is too much already. @dodgex WDYT?

@smaugho
Copy link
Contributor Author

smaugho commented Feb 17, 2019

Well, I guess that what I'm proposing could be applied to the Activity scope as well, probably another annotation would be better for this.

But well, with fragments is quite common scenario, that you have a Fragment, holding a ViewPager, which hold other fragments.. there it would be really useful. Basically we are extending the scope, permitting to the programmer define, in what scope a given Bean is valid.

Well, if @dodgex considers it is useful, we could go with another annotation for it, maybe simply @Scope, which has several options "tags, ids, classes".. It would apply to the fragments or even different activities (by class).

Note that in the current implementation, is not that much, basically we need "to find the bean holder", that's all, everything else works like in #2206

@dodgex
Copy link
Member

dodgex commented Feb 18, 2019

After reading the issue for like 3 times i think i understand what you want.

I agree with @WonderCsabo that it would overload the @EBean annotation. but I'm not sure how possible it is to have a second variable that limits where it might be injected. that would require the @Bean annotation handler to take care of the second annotation too.

@smaugho
Copy link
Contributor Author

smaugho commented Feb 18, 2019

On this case is the @EBean which should take care of the other annotation.. But well, there are several cases like this already on the code.. For instance, the @EActivity should take care of the @DataBound. So the @EBean should take care for this case of the @Scope

@dodgex
Copy link
Member

dodgex commented Feb 18, 2019

@smaugho don't you want to limit where the bean can be injected? imho this needs to be handled by the injectin annotation and thus it should be done in @Bean or am i missing something?

@smaugho
Copy link
Contributor Author

smaugho commented Feb 18, 2019

@dodgex the injection is really "handled" on the @EBean.. The new BeanHolder mechanism is implemented like this, for instance, for the Activity scope:

    public static ActivityScopedBean_ getInstance_(Context context) {
        if (context instanceof BeanHolder) {
            BeanHolder beanHolder = ((BeanHolder) context);
            ActivityScopedBean_ bean = beanHolder.getBean(ActivityScopedBean_.class);
            if (bean == null) {
                bean = new ActivityScopedBean_(context);
                beanHolder.putBean(ActivityScopedBean_.class, bean);
                bean.init_();
            }
            return bean;
        }
        ActivityScopedBean_ bean = new ActivityScopedBean_(context);
        bean.init_();
        return bean;
    }

So, basically the proposed @Scope annotation what should do here is to find "the right BeanHolder. Something like:

    public static ActivityScopedBean_ getInstance_(Context context) {
        BeanHolder beanHolder  = findBeanHolder(context);
        //Do the injection from beanHolder if found
        //default to normal constructor
    }

So basically, what @Scope would do, is to implement this findBeanHolder().

@smaugho
Copy link
Contributor Author

smaugho commented Feb 18, 2019

So, @Bean continues calling the normal getInstance_ method like before

@dodgex
Copy link
Member

dodgex commented Feb 18, 2019

okay. you are right with this. sorry. :)

but honestly i think if possible the scope should be checked while generating the injecting code to validate if a injection is valid at compile time rather than on runtime.

e.g. fail if you try to inject a fragment scoped bean in an activity or some other bean. or if it is injected into another bean and the other bean is not scoped [the same way].

@WonderCsabo wdyt?

@smaugho
Copy link
Contributor Author

smaugho commented Feb 18, 2019

Well, this would be a validation, which can be done on the @Bean.. That would not change the current code I think. I think the validations are quite valid to do them in the BeanHandler.. If @WonderCsabo agrees with it, I can do them. Note that there are 2 scenarios:

  1. Bean injected on the Activity or Fragment being scoped with that.
  2. Bean injected in another bean.. in that case, the Scopes should be the same then.

@WonderCsabo
Copy link
Member

@smaugho if it can be done without adding too much complexity, let's do it.

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