Android widget code

Question

Anyone have any comments? Just trying to better my code if can be done.

public class SacWidget extends AppWidgetProvider {
 String roseUrl;
 AQuery aq;
 public void onUpdate(Context context, AppWidgetManager appWidgetManager,
 int[] appWidgetIds) {
 final int N = appWidgetIds.length;
 for (int i = 0; i < N; i++) {
 int appWidgetId = appWidgetIds[i];
 Intent intent = new Intent(context, DangerRose.class);
 PendingIntent pendingIntent = PendingIntent.getActivity(context, 0,
 intent, 0);
 RemoteViews views = new RemoteViews(context.getPackageName(),
 R.layout.widget_layout);
 views.setImageViewBitmap(R.id.ivdangerrose, getRose());
 appWidgetManager.updateAppWidget(appWidgetId, views);
 }
 }
 private Bitmap getRose() {
 Bitmap bitmap = null;
 File f = new File("/storage/emulated/0/acr/sac/dangerrose.png");
 try {
 bitmap = BitmapFactory.decodeStream(new FileInputStream(f));
 } catch (FileNotFoundException e) {
 e.printStackTrace();
 }
 return bitmap;
 }
}

Answer 1

I'm not too familiar with Android, so the following is just some generic Java notes:

1. Fields roseUrl and aq seem unused as well as the pendingIntent local variable. You might remove them.

2. You could use a foreach loop:

   for (final int appWidgetId: appWidgetIds) ...
   

3. It isn't the best idea to use printStackTrace() in Android exceptions.

4. The getRose() method creates a stream (new FileInputStream(f)). I'm not sure whether BitmapFactory.decodeStream closes it before it returns or not. If not, you should close it.

Notes for the edit:

1. You should close the stream in a finally block. If BitmapFactory.decodeStream throws an exception it won't be closed. See Guideline 1-2: Release resources in all cases in Secure Coding Guidelines for the Java Programming Language

2. The following two lines are duplicated:

   File ext = Environment.getExternalStorageDirectory();
   File file = new File(ext,"acr/sac/dangerrose.png");
   

   You could extract them out to a method.

• jasonflaherty: Thanks for the answer, I really appreciate it. I cleaned up the code a bit. I cannot find a close for the IS in any of the documentation.... hmmmm....

Answer 2

1. Bad variable names. Please do not use things like N, i, and f (and if you are, at least be consistent and make them all lowercase). That aq is questionable as well.

2. Using for (int appWidgetId : appWidgetIds) would save you a couple lines, and get rid of that atrocious i and N variables as well.

3. Personally I wouldn't bother declaring N as final (or at all, since you could just as easily use appWidgetIds.length in your for loop condition even if you don't do #2) unless there is reason to. Like if you're going to refer to it from an anonymous class. Note that if you do #2 above, N goes away completely.

4. Close your FileInputStream. Instead of doing File f = ... do FileInputStream input = new FileInputStream(new File("/storage/...")); and then input.close(); when you are done with it.