I recently developed my first appwidget for Android, and it works really nice. I basically created a collection widget and use a Loader to retrieve the data from a database, and when a user clicks on the list item, that items details will be saved into a bundle in an intent, and the main activity will be launched with the bundled info displayed to the user.
My code works as intended; however, I am concerned with possible edge cases in how widgets actually work, initialize, and update that I have not thought of. Again, I am new to widget development, and the API for widgets took a lot for me to wrap my head around.
Question Objective
While any code review is helpful, I am basing my question on the premise set up here to answer #1 and #2 from the reviewer's point of view:
- Does this code actually do what it is supposed to do under normal conditions?
- Are there edge cases where this code will fail?
This GitHub link links directly to my Hub Flavor that implements the appwidget (Java and res folders, and manifest file).
App Widget Provider
public class ResumeHubWidgetProvider extends AppWidgetProvider {
private static final String LAUNCH_RESUME_ACTION = "io.github.ciscorucinski.personal.intro.hub.LAUNCH_RESUME_ACTION";
private static Intent createIntent(Context context) {
return new Intent(context, ResumeHubWidgetProvider.class);
}
private static void updateAppWidget(Context context, AppWidgetManager appWidgetManager,
int appWidgetId) {
Intent serviceIntent = MyWidgetService.createIntent(context);
serviceIntent.putExtra(AppWidgetManager.EXTRA_APPWIDGET_ID, appWidgetId);
serviceIntent.setData(Uri.parse(serviceIntent.toUri(Intent.URI_INTENT_SCHEME)));
// Construct the RemoteViews object
RemoteViews views = new RemoteViews(context.getPackageName(), R.layout.resume_hub_list_widget);
views.setRemoteAdapter(R.id.widget_list, serviceIntent);
Intent widgetProviderIntent = ResumeHubWidgetProvider.createIntent(context);
widgetProviderIntent.setAction(ResumeHubWidgetProvider.LAUNCH_RESUME_ACTION);
widgetProviderIntent.putExtra(AppWidgetManager.EXTRA_APPWIDGET_ID, appWidgetId);
serviceIntent.setData(Uri.parse(serviceIntent.toUri(Intent.URI_INTENT_SCHEME)));
PendingIntent pendingIntent = PendingIntent.getBroadcast(context, 0, widgetProviderIntent,
PendingIntent.FLAG_UPDATE_CURRENT);
views.setPendingIntentTemplate(R.id.widget_list, pendingIntent);
// Instruct the widget manager to update the widget
appWidgetManager.updateAppWidget(appWidgetId, views);
}
@Override
public void onUpdate(Context context, AppWidgetManager appWidgetManager, int[] appWidgetIds) {
// There may be multiple similar widgets active, so update all of them
for (int appWidgetId : appWidgetIds) {
updateAppWidget(context, appWidgetManager, appWidgetId);
}
}
@Override
public void onReceive(Context context, Intent intent) {
AppWidgetManager appWidgetManager = AppWidgetManager.getInstance(context);
if (intent.getAction().equals(LAUNCH_RESUME_ACTION)) {
Timber.i("Intent Action is LAUNCH_RESUME_ACTION");
// Open the Resume activity with the user selected resume info
Bundle bundle = intent.getBundleExtra(CREATE_INTENT_BUNDLE);
context.startActivity(ResumeActivity
.createIntentWithFlags(context, bundle,
Intent.FLAG_ACTIVITY_NEW_TASK));
}
int appWidgetIds[] = appWidgetManager.getAppWidgetIds(new ComponentName(context, ResumeHubWidgetProvider.class));
appWidgetManager.notifyAppWidgetViewDataChanged(appWidgetIds, R.id.widget_list);
super.onReceive(context, intent);
}
}
Adapter
class HubWidgetAdapter implements RemoteViewsService.RemoteViewsFactory,
Loader.OnLoadCompleteListener<List<Resume.People>> {
private Context context;
private List<Resume.People> data;
private int appWidgetId;
private PeopleLoader loader;
HubWidgetAdapter(Context context, Intent intent) {
this.context = context;
appWidgetId = intent.getIntExtra(AppWidgetManager.EXTRA_APPWIDGET_ID,
AppWidgetManager.INVALID_APPWIDGET_ID);
}
@Override
public RemoteViews getViewAt(int position) {
RemoteViews view = new RemoteViews(context.getPackageName(),
android.R.layout.simple_list_item_1);
Resume.People person = data.get(position);
view.setTextViewText(android.R.id.text1, person.seeking_position());
view.setTextColor(android.R.id.text1, Color.BLACK);
Bundle bundle = new Bundle();
bundle.putLong(ResumeActivity.ID, person._id());
bundle.putString(ResumeActivity.NAME, person.name());
bundle.putString(ResumeActivity.EMAIL, person.email());
bundle.putString(ResumeActivity.PHONE, person.phone());
bundle.putString(ResumeActivity.GITHUB, person.github());
bundle.putString(ResumeActivity.LINKEDIN, person.linkedin());
bundle.putString(ResumeActivity.SEEKING, person.seeking_position());
Intent intent = new Intent();
intent.putExtra(CREATE_INTENT_BUNDLE, bundle);
Timber.i("Bundled Person%s", intent);
view.setOnClickFillInIntent(android.R.id.text1, intent);
return view;
}
@Override
public void onLoadComplete(Loader<List<Resume.People>> loader, List<Resume.People> data) {
this.data = data;
}
@Override
public void onCreate() {
data = new ArrayList<>();
}
@Override
public void onDestroy() {
if (loader != null) {
loader.unregisterListener(this);
loader.cancelLoad();
loader.stopLoading();
loader = null;
}
data = null;
}
@Override public void onDataSetChanged() {
loader = new PeopleLoader(context);
loader.registerListener(0, this);
loader.startLoading();
}
@Override public int getCount() { return data.size(); }
@Override public RemoteViews getLoadingView() { return null; }
@Override public int getViewTypeCount() { return 1; }
@Override public long getItemId(int position) { return position; }
@Override public boolean hasStableIds() { return true; }
}
Service
public class MyWidgetService extends RemoteViewsService {
static Intent createIntent(Context context) {
return new Intent(context, MyWidgetService.class);
}
@Override
public RemoteViewsFactory onGetViewFactory(Intent intent) {
return new HubWidgetAdapter(this.getApplicationContext(), intent);
}
}
Manifest (Hub Flavor)
The Hub flavor is the only aspect of my code that has appwidgets, so I am only including the Hub Manifest file here that is merged into the main Manifest file.
<manifest xmlns:android="http://schemas.android.com/apk/res/android">
<application>
<activity
android:name="io.github.ciscorucinski.personal.intro.ui.ResumeActivity"
android:label="@string/app_name"
android:theme="@style/AppTheme.NoActionBar"/>
<receiver android:name="io.github.ciscorucinski.personal.intro.hub.ResumeHubWidgetProvider">
<intent-filter>
<action android:name="android.appwidget.action.APPWIDGET_UPDATE" />
</intent-filter>
<meta-data
android:name="android.appwidget.provider"
android:resource="@xml/resume_hub_list_widget_info" />
</receiver>
<service
android:name="io.github.ciscorucinski.personal.intro.hub.MyWidgetService"
android:exported="false"
android:permission="android.permission.BIND_REMOTEVIEWS" />
</application>
</manifest>
-
\$\begingroup\$ The edits you did are good. What's missing is a good title. The title of a question should be a one-line summary of what the code does. See also the how-to-ask page \$\endgroup\$janos– janos2016年07月01日 07:49:09 +00:00Commented Jul 1, 2016 at 7:49
-
\$\begingroup\$ No code changes, but the code now works? What? \$\endgroup\$Pimgd– Pimgd2016年07月01日 10:28:30 +00:00Commented Jul 1, 2016 at 10:28
-
\$\begingroup\$ "Does this code actually do what it is supposed to do under normal conditions?" - While a review might point out issues that you could have missed, Code Review is not a site that tests the normal operation of your code for you. \$\endgroup\$SuperBiasedMan– SuperBiasedMan2016年07月01日 14:46:43 +00:00Commented Jul 1, 2016 at 14:46
-
\$\begingroup\$ Yes, under normal conditions, my code works as intended. I can do everything that I intended the widget to do; nothing more, nothing less. In fact, I was using this code base for around 5 days already and then uploaded it to github. But, I am concerned with edge cases. By the way, I always said my code works, someone decided to mark this question as not fit for this site solely on my bad wording of the original question, and then are confused as to why my code was not changed. It's not changed because it works! \$\endgroup\$Christopher Rucinski– Christopher Rucinski2016年07月01日 14:53:54 +00:00Commented Jul 1, 2016 at 14:53
-
\$\begingroup\$ @SuperBiasedMan I got #1 and #2 points from the user RubberDuck and his linked response to what is a code review on this website. So you will have to talk to RubberDuck about being wrong. \$\endgroup\$Christopher Rucinski– Christopher Rucinski2016年07月01日 14:59:54 +00:00Commented Jul 1, 2016 at 14:59
1 Answer 1
While this answer will not fulfil the question's objectives, it will provide a general-level review intended to make the code more readable. Also, it will display my current iteration of the code.
First, if you look at ResumeHubWidgetProvider::updateAppWidget(...)
in regards to the widgetProviderIntent
variable, you will see a static method call (within ResumeHubWidgetProvider
) to create an empty Intent. Right after that, that intent is given an Action
specified by a constant variable in ResumeHubWidgetProvider
. So there appears to be so refactoring that can be done...
First, I refactored ResumeHubWidgetProvider::createIntent(...)
to return a "fully-packaged" and complete Intent
. Second, I added ResumeHubWidgetProvider::createPendingIntent(...)
to return a fully-packaged and complete PendingIntent
. Third, I refactored MyWidgetService::createIntent(...)
to return a fully-packaged and complete Intent. Lastly, I moved the creation of the RemoteViews
after the creation of all the Intents
. That gave me a method that is a lot easier to understand...
private static void updateAppWidget(Context context, AppWidgetManager manager, int widgetId) {
// Create needed intents
Intent intent = ResumeHubWidgetProvider.createActionIntent(context, widgetId);
PendingIntent pendingIntent = ResumeHubWidgetProvider.createPendingIntent(context, intent);
Intent serviceIntent = MyWidgetService.createIntent(context, widgetId);
// Construct the RemoteViews object
RemoteViews views = new RemoteViews(context.getPackageName(), R.layout.resume_list_widget);
views.setRemoteAdapter(R.id.widget_list, serviceIntent);
views.setPendingIntentTemplate(R.id.widget_list, pendingIntent);
// Instruct the widget manager to update the widget
manager.updateAppWidget(widgetId, views);
}
As you can see, I also refactored some of the names of some items compared to my original source to try to keep my code under 80?? characters per line.
Anyways, here is the full code. As you will see, I did some more refactoring elsewhere in the class, too. Note: the Manifest
file and Adapter
class have not changed.
ResumeHubWidgetProvider
public class ResumeHubWidgetProvider extends AppWidgetProvider {
private static final String LAUNCH_RESUME_ACTION = "io.github.ciscorucinski.personal.intro.hub.LAUNCH_RESUME_ACTION";
private static Intent createActionIntent(Context context, int widgetId) {
Intent intent = new Intent(context, ResumeHubWidgetProvider.class);
intent.setAction(ResumeHubWidgetProvider.LAUNCH_RESUME_ACTION);
intent.putExtra(AppWidgetManager.EXTRA_APPWIDGET_ID, widgetId);
return intent;
}
private static PendingIntent createPendingIntent(Context context, Intent intent) {
return PendingIntent.getBroadcast(context, 0, intent, PendingIntent.FLAG_UPDATE_CURRENT);
}
private static void updateAppWidget(Context context, AppWidgetManager manager, int widgetId) {
// Create needed intents
Intent intent = ResumeHubWidgetProvider.createActionIntent(context, widgetId);
PendingIntent pendingIntent = ResumeHubWidgetProvider.createPendingIntent(context, intent);
Intent serviceIntent = MyWidgetService.createIntent(context, widgetId);
// Construct the RemoteViews object
RemoteViews views = new RemoteViews(context.getPackageName(), R.layout.resume_list_widget);
views.setRemoteAdapter(R.id.widget_list, serviceIntent);
views.setPendingIntentTemplate(R.id.widget_list, pendingIntent);
// Instruct the widget manager to update the widget
manager.updateAppWidget(widgetId, views);
}
@Override
public void onUpdate(Context context, AppWidgetManager appWidgetManager, int[] appWidgetIds) {
// There may be multiple similar widgets active, so update all of them
for (int appWidgetId : appWidgetIds) {
updateAppWidget(context, appWidgetManager, appWidgetId);
}
}
@Override
public void onReceive(Context context, Intent intent) {
AppWidgetManager appWidgetManager = AppWidgetManager.getInstance(context);
switch (intent.getAction()) {
case LAUNCH_RESUME_ACTION:
executeResumeAction(context, intent);
break;
default: // do nothing
}
int appWidgetIds[] = ResumeHubWidgetProvider.getAppWidgetIds(context, appWidgetManager);
appWidgetManager.notifyAppWidgetViewDataChanged(appWidgetIds, R.id.widget_list);
super.onReceive(context, intent);
}
private void executeResumeAction(Context context, Intent bundledIntent) {
Timber.i("Intent Action is LAUNCH_RESUME_ACTION");
Bundle bundledPerson = bundledIntent.getBundleExtra(CREATE_INTENT_BUNDLE);
Intent launchActivityIntent = ResumeActivity
.createFlagIntent(context, bundledPerson, Intent.FLAG_ACTIVITY_NEW_TASK);
context.startActivity(launchActivityIntent);
}
private static int[] getAppWidgetIds(Context context, AppWidgetManager manager) {
return manager.getAppWidgetIds(new ComponentName(context, ResumeHubWidgetProvider.class));
}
}
MyWidgetService
public class MyWidgetService extends RemoteViewsService {
static Intent createIntent(Context context, int appWidgetId) {
Intent serviceIntent = new Intent(context, MyWidgetService.class);
serviceIntent.putExtra(AppWidgetManager.EXTRA_APPWIDGET_ID, appWidgetId);
serviceIntent.setData(Uri.parse(serviceIntent.toUri(Intent.URI_INTENT_SCHEME)));
return serviceIntent;
}
@Override
public RemoteViewsFactory onGetViewFactory(Intent intent) {
return new HubWidgetAdapter(this.getApplicationContext(), intent)
}
}