Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

Is this a good technique (Using data holder) to eliminate anonymous class, to reduce memory leak risk

Tags:

java

android

Anonymous class can easily cause memory leak, especially in Android world, where Activity or Fragment can suddenly destroyed due to configuration changes. Here are one of many examples.

http://chaosinmotion.com/blog/?p=696

http://blog.andresteingress.com/2011/10/12/anonymous-inner-classes-in-android/

https://blogs.oracle.com/olaf/entry/memory_leaks_made_easy

The reason is that, creating an anonymous class in Activity or Fragment, the anonymous class will always hold an implicit reference to the Activity or Fragment. So, when Activity tends to go out of life due to configuration changes, it cannot be garbage collected if the anonymous class is being exposed and hold by outside world.

So, I was wondering, whether using a data holder technique is a good way, to completely eliminate anonymous class, to reduce risk of memory leakage? Or, am I over paranoid?

Using anonymous class

public class HomeMenuFragment {
    private Parcelable selectedInfo = null;
    private List<View> homeMenuRows = new ArrayList<View>();

    private void fun() {
        ...
        ...
        row.setOnClickListener(new OnClickListener() {

            @Override
            public void onClick(View arg0) {
                // homeMenuRows is member variable
                for (View r : homeMenuRows) {
                    r.setSelected(false);
                }
                row.setSelected(true);
                // selectedInfo is member variable
                selectedInfo = watchlistInfo;
            }
        });
    }
}

Refactor using data holder technique

public class HomeMenuFragment {
    private static class Holder {
        public Parcelable selectedInfo = null;
        public final List<View> homeMenuRows = new ArrayList<View>();        
    }
    private final Holder holder = new Holder();

    private static class MyOnClickLisnter implements OnClickListener {
        private final Holder holder;
        private final LinearLayout row;
        private final WatchlistInfo watchlistInfo;

        public MyOnClickLisnter(Holder holder, LinearLayout row, WatchlistInfo watchlistInfo) {
            this.holder = holder;
            this.row = row;
            this.watchlistInfo = watchlistInfo;
        }

        @Override
        public void onClick(View arg0) {
            for (View r : holder.homeMenuRows) {
                r.setSelected(false);
            }
            row.setSelected(true);
            holder.selectedInfo = watchlistInfo;
        }        
    }

    private void fun() {
        ...
        ...
        row.setOnClickListener(new MyOnClickLisnter(holder, row, watchlistInfo));
    }
}
like image 551
Cheok Yan Cheng Avatar asked Mar 20 '13 15:03

Cheok Yan Cheng


1 Answers

Refactoring like this does not help. The View objects in the Holder also hold references to the Activity, so if the Holder is exposed to the outside world you have a memory leak. Also, your MyOnClickLisnter instance, even though it is declared static, is holding an explicit reference to the Holder and a LinearLayout, both of which are holding references to the Activity. I'm not a fan of anonymous classes, but I think the chances of an anonymous OnClickListener being passed outside of the Activity or Fragment are very very small. Sounds to me like you are being overly paranoid.

like image 54
David Wasser Avatar answered Nov 20 '22 21:11

David Wasser