Is there anything wrong with static field initialised with anonymous inner class?

Question

I found this code in old legacy system and I don't want to touch it, if there is nothing wrong with it.

But I feel like this code has some flaw and I can't locate it. Is this just not common pattern or there is some hidden pitfalls or memory leaks?

private static final Foo action =  new Foo() {

    @Override
    public void onAction(MyDialogFragment fragment) {
        if (fragment.getContext() != null) {
            fragment.getActivity().finish();
        }
    }
};

This code is used in a fragment

UPDATE: I suspect that an object of anonymous inner class has a reference to a parent fragment and since an object of this class saved in static field, it will never be collected, so parent fragment will never be collected too. Are my reasonings wrong?

I'd probably use `Consumer consumer = frag -> {};`. I'd also question whether this needs to be `static`. — PPartisan, Sep 20 '18 at 16:06

score 3 · Answer 1 · answered Sep 20 '18 at 16:26

3

There is nothing wrong with the approach, since the logic deals only with the values passed as parameters. Using a static simply avoids creating multiple instances of the Foo class.

answered Sep 20 '18 at 16:26

Steve11235

2,849
1
17
18

Thanks, I was thinking that an object of anonymous inner class has a reference to a parent fragment and since an object of this class saved in static field, it will never be collected, so parent fragment will never be collected too. Are my reasonings wrong? – Maksim Turaev Sep 21 '18 at 06:16

score 1 · Answer 2 · answered Sep 20 '18 at 15:52

1

Yes there is. It's better to get the fragment like this:

MyDialogFragment fragment = MyDialogFragment.instance();
fragment.setNavigator(this);
getSupportFragmentManager().beginTransaction().replace(R.id.activity_myDialog_layout, fragment).commit();

There is a better way. You can create 3 classes and an interface which will be implemented by the 3 classes. Activity, Fragment and Presenter. The interface will be Contracts. This is more advanced.

answered Sep 20 '18 at 15:52

siqqQ

43
1
10

Sure I can, but I found this in legacy system and I don't want to touch it if there is nothing wrong here. If you could explain what is the problem with this approach, please do. – Maksim Turaev Sep 20 '18 at 16:08
If it is in legacy system it's normal. – siqqQ Sep 22 '18 at 08:16

score 0 · Accepted Answer · answered Nov 26 '19 at 06:06

0

Best explanation is here: https://stackoverflow.com/a/27739694/5868421

Anonymous class in static context don't hold a reference to outer object.

answered Nov 26 '19 at 06:06

Maksim Turaev

4,115
1
29
42

Is there anything wrong with static field initialised with anonymous inner class?

3 Answers3