0

I would like to know whether it's a bad design choice to use a singleton MainAcitivity as in the following:

public class MainActivity extends AppCompatActivity ... {
   public static MainActivity mainActivitySingleton;
   ....
   @Override
   protected void onCreate(Bundle savedInstanceState) {
   mainActivitySingleton=this;

For instance, in many occasions where I need to access the context I use getContext(), but sometimes (I don't know why) getContext() returns null resulting in a runtime exception. I ended up using the mainActivitySingleton I created instead of getContext().

My little finger tells me this is a bad design choice! if it is the case, can anyone explain why?

aminography
  • 21,986
  • 13
  • 70
  • 74
Abdu
  • 487
  • 4
  • 12
  • you are making a memory leak this way better to solve the null context than this way – Oussema Aroua Dec 14 '18 at 14:23
  • Use **Abstract base class** as parent activity instead. – Jeel Vankhede Dec 14 '18 at 14:23
  • resulting in memory leaks. Where do you call getContext() resulting in null? – Christoph Mayr Dec 14 '18 at 14:36
  • 1
    Its generally a bad idea to make singletons out of Activities. If you require a context in a Fragment where it might be null, Android Support Library 27.1.0 has new methods requireActivity(), requireContext(). – Timay Dec 14 '18 at 14:54
  • @ChristophMayr I noticed that getContext() loses context and returns null in callbacks. Why?! – Abdu Dec 14 '18 at 20:10
  • That is not creating a singleton `MainActivity`. That's just assigning the current `MainActivity` instance to a static field. The system is still creating new, separate instances as needed, unless you've used an appropriate `launchMode` in the manifest, in which case there's no need to attempt to create a singleton yourself. – Mike M. Dec 15 '18 at 01:33

2 Answers2

3

Holding a static reference to Activity object or Context, is one of the memory leak cases which leads to extra memory consumption then performance lost. If no reference points to an object, then it is marked as a candidate to be garbage collected. When an object is not used anymore in the program but its memory cannot be released by the Garbage Collector, it is considered as a memory leak case. Therefore, in case of static reference to an Activity, its memory could not be freed after calling onDestroy method.

If you really want to hold a static reference to an instance of Activity or Context, it is recommended to use WeakReference.

public class MyActivity extends AppCompatActivity {

    private static WeakReference<Context> sContextReference;

    @Override 
    protected void onCreate(Bundle savedInstanceState) {
        super.onCreate(savedInstanceState);
        setContentView(R.layout.activity_main);
        sContextReference = new WeakReference<>(this);
    }
} 

Usage:

Context context = sContextReference.get();

.

UPDATE:

Better solution to retain a reference to an instance of Context is to create and hold a weak reference to it in an Application class. By this way, we sure that only one reference is pointed to the application context during run time of the app.

MyApplication.java

import android.app.Application;
import android.content.Context;

import java.lang.ref.WeakReference;

public class MyApplication extends Application {

    private static WeakReference<Context> sContextReference;

    @Override
    public void onCreate() {
        super.onCreate();
        sContextReference = new WeakReference<>(getApplicationContext());
    }

    @Override
    public void onTerminate() {
        super.onTerminate();
        sContextReference.clear();
    }

    public static Context getContext() {
        return sContextReference.get();
    }

}

manifest.xml

<application
    android:name="path.to.MyApplication"
    android:icon="@mipmap/ic_launcher"
    android:label="@string/app_name"
    android:roundIcon="@mipmap/ic_launcher_round"
    android:theme="@style/AppTheme.NoActionBar">

    ...

</application>

Usage:

Context context = MyApplication.getContext();
aminography
  • 21,986
  • 13
  • 70
  • 74
  • Ok. I understand the pb with memory leaks. However, How about if someone say: there is only one instance of my app (hence only one instance of MainActivity object), then I might think that it's not that of a big deal if memory is not freed after calling `onDestroy`. Afterall, when a new MainActivity object is created, then the singleton reference will refer to this new object, then the old object is not referenced anymore and hence can be freed!"? – Abdu Dec 14 '18 at 20:03
  • Although `MainActivity` may be created one time when your app is running (if we declare it as `singleInstance` in manifest, we sure that is single object). When your app goes to background by pressing back button, if you are using referenced context in background threads, then its instance would not be released. By launching the app another time, new instance of `MainActivity` is created and previous instance is also retain in memory. After all this instance may cause many runtime problems, because it is detached from window manager and so on... – aminography Dec 15 '18 at 05:09
  • However the better solution is to retain a weak reference to the application context in an application class. Because we sure that only one instance of application class is created during run time of the app. I've updated the answer. – aminography Dec 15 '18 at 05:16
  • not yet, thank you for the hint to weakreference. In my use case, I don't necessarily need a "perpetuate" reference to my MainActivity object, but i actually needed it mainly for the context. @Dev.Hrant's of getting the context in `onActivityCreated` seems ok for now. – Abdu Dec 15 '18 at 23:26
1

Never do that. That is the bad design pattern. You should not save activity instance as static instance, because that potential memory leak. If you call getContext() method in fragment instance then you should call getContext() in this lifecircle onActivityCreated() method.

Like this:

@Override
public void onActivityCreated(@Nullable Bundle savedInstanceState) {
    super.onActivityCreated(savedInstanceState);
    Context context = getContext();
}
Rid Hrant
  • 112
  • 1
  • 1
  • 11
  • is this the best way to keep track of the context? what about @aminography suggestion to keep a static weak reference? and why in the earth getContext() loses context and returns null (I noticed this happens in callbacks)?! – Abdu Dec 14 '18 at 20:08
  • @Abdu Why you want to save activity instance? please say your usecase. – Rid Hrant Dec 14 '18 at 20:53
  • as I explained in my post, I noticed that I get runtime error because of `getContext()` returning `null` in some cases (mainly in http get callbacks). So I started using the singleton as the context which works fine. But as I said I suspect this to be a bad design pattern. now that you have confirmed my suspicion, I need to understand why getContext() returns null, so that I can fix the problem.. thank you – Abdu Dec 15 '18 at 00:13
  • here is an explanation of why the getContext() might return `null` (losing context within a callback in a fragment) https://stackoverflow.com/questions/47987649/why-getcontext-in-fragment-sometimes-returns-null. Basically if the user leaves the fragment `onDetach()` is called resulting in the loss of the context. – Abdu Dec 15 '18 at 23:34