I recently had to work in a tricky piece of work for a project I am working on. You can find its code on GitHub and I'd like to have a feedback about some specific parts of it.
The project is a library and if you look at the README you will see how it needs to be integrated with an android application. I report it here:
public class YOUR_APP_NAMEApplication extends Application{
private RateMyApp rateMyApp;
@Override
public void onCreate(){
RateMyAppBuilder builder = new RateMyAppBuilder();
builder.setLaunchesBeforeAlert(3);
builder.setDaysBeforeAlert(7);
builder.setEmailAddress("[email protected]");
PreferencesManager preferencesManager = SharedPreferencesManager.buildFromContext(this);
rateMyApp = builder.build(preferencesManager);
}
public void activityCreated(Activity activity){
if(rateMyApp != null)
rateMyApp.appLaunched(activity);
}
public void activityDestroyed(){
if(rateMyApp != null)
rateMyApp.activityDestroyed();
}
}
public class MainActivity extends Activity {
@Override
public void onCreate(Bundle savedInstanceState) {
setContentView(R.layout.activity_main);
super.onCreate(savedInstanceState);
FutsalCoachApplication application = (FutsalCoachApplication) getApplicationContext();
application.activityCreated(this);
}
@Override
public void onDestroy(){
FutsalCoachApplication application = (FutsalCoachApplication) getApplicationContext();
application.activityDestroyed();
super.onDestroy();
}
}
The first thing I'd like to ask is if it acceptable to require the creation of a new class extending Application
just to hold the main component of my library.
I'd also like to get a feedback about the fact that my library needs to create instances of DialogFragment
and to start new activities. Both these tasks require an active Activity
and this is a source of a bit of trickery.
I like the fact that RateMyApp
class has the dependency on the Activity
instance isolated in the RateMyApp.appLaunched(Activity)
method.
I don't really like how I get the class extending Application
to get a reference of the current Activity
. Needing to override Activity.onCreate(Bundle)
and Activity.onDestroy()
feels a bit dirty.
What do you think about this approach? Would you do it in an alternative (and possible cleaner) way?
1 Answer 1
First, I would recommend that you take a look at the null object pattern to avoid the != null
check here:
if (rateMyApp != null)
rateMyApp.appLaunched(activity);
By using a "null object", simply rateMyApp.appLaunched(activity);
would be enough. Your null object is an instance of a special class that just doesn't do anything in the appLaunched
method.
The first thing I'd like to ask is if it acceptable to require the creation of a new class extending Application just to hold the main component of my library.
No. Absolutely not. I don't really see why you are using the Application
to do that. You could write a regular Java class for holding the main component of your library. Or rather, you could let the user of your library determine how to hold the main component of your library.
Instead of using the onCreate
method of the Application
, you could let the user initialize the RateMyApp
object in the onCreate
of a regular Activity
.
I'd also like to get a feedback about the fact that my library needs to create instances of DialogFragment and to start new activities. Both these tasks require an active Activity and this is a source of a bit of trickery.
Technically, those tasks don't need an active Activity. They need a Context
. However, "acquiring" an activity should not be that much of a problem. See below.
I don't really like how I get the class extending Application to get a reference of the current Activity.
Me neither. Why does it need a reference of the current activity?
Would you do it in an alternative (and possible cleaner) way?
Let the user whenever he likes create a RateMyApp
object, and call the .appLaunched
method on it (as you've done in the sample). There is no need for your RateMyApp
to know which is the current showing activity, it only needs to know about an activity (or rather, Context
) when it should do something - i.e. possibly show a dialog or read/write the preferences to keep track on how many times/days the application has been used. When the user of your library wants your library to possibly show the dialog, let it call a method on your library and pass along the current Activity
as a parameter. That's it. No need to make things more complicated than necessary.
Summary
By extending Application as you've done in your question, you have only added a middle hand that doesn't add any value at all. All the things that are done there can, and in this case really should, be done in the MainActivity
instead.
-
1\$\begingroup\$ Thanks for your answer. Actually the real issue was another. The reason I used the class extending
Application
was to discriminate between the creation of anActivity
at the startup of the application and the creation of the activity for another reason. I then discovered, as you can see in the sample, that the parameter ofonCreate
is null when the activity is created for the first time and I used this knowledge to get rid of the class extendingApplication
\$\endgroup\$mariosangiorgio– mariosangiorgio2014年04月24日 20:40:21 +00:00Commented Apr 24, 2014 at 20:40 -
\$\begingroup\$ Minor note: As to the first point,
rateMyApp
will never be null, so the null check is just unnecessary anyway. (Application.onCreate() will always be called before any activities are created) \$\endgroup\$Kevin Coppock– Kevin Coppock2014年05月01日 06:31:00 +00:00Commented May 1, 2014 at 6:31
RateMyApp
class to be reviewed? If so, add that class to your question. \$\endgroup\$