8
\$\begingroup\$

I am building an Android application and there are five Activity classes or if you're familiar with the MVC pattern they would usually be the Controller classes.

Specifically a User will enter one of these 5 Activity classes (by navigating throughout the app) and sometimes they might upload a photo. Now the code for uploading a photo follows a very similar pattern. Please note all this code is repeated 5 times in all 5 classes (YUCK).

Global Variables:

/*
Tracking
 */
private static final int TAKE_PHOTO_REQUEST = 1;
private static final int GET_FROM_GALLERY = 2;

private Uri mUri;
private String mCurrentPhotoPath;
private File mFile;
private TypedFile mTypedFile; // For Retrofit

User hits Photo Upload Button, and a AlertDialog pops up:

private void showFileOptions() {
    new AlertDialog.Builder(this)
            .setItems(R.array.uploadOptions, new DialogInterface.OnClickListener() {
                @Override
                public void onClick(DialogInterface dialog, int which) {
                    switch (which) {
                        case 0:
                            dispatchTakePicture();
                            break;
                        case 1:
                            dispatchUploadFromGallery();
                            break;
                    }
                }
            })
            .setNegativeButton(android.R.string.cancel, new DialogInterface.OnClickListener() {
                @Override
                public void onClick(DialogInterface dialog, int which) {
                    dialog.cancel();
                }
            })
            .show();
}

dispatchTakePicture:

/*
Take picture from your camera
 */
private void dispatchTakePicture() {
    Intent intent = new Intent(MediaStore.ACTION_IMAGE_CAPTURE);
    // Make sure that there is a camera activity to handle the intent
    if (intent.resolveActivity(getPackageManager()) != null) {

        // Create the File where the mTypedFile would go
        File picFile = null;

        try {
            picFile = createImageFile();
            mFile = picFile;
        } catch (IOException e) {
            e.printStackTrace();
            Toast.makeText(this, e.getMessage(), Toast.LENGTH_LONG).show();
        }

        // Continue only if the file was successfully created
        if (picFile != null) {
            intent.putExtra(MediaStore.EXTRA_OUTPUT, Uri.fromFile(picFile));
            startActivityForResult(intent, TAKE_PHOTO_REQUEST);
        }
    }
}

dispatchUploadFromGallery:

/*
Take a mTypedFile from your gallery
 */
private void dispatchUploadFromGallery() {
    // Launch gallery intent
    startActivityForResult(new Intent(Intent.ACTION_PICK, MediaStore
            .Images.Media.INTERNAL_CONTENT_URI), GET_FROM_GALLERY);
}

Please note that startActivityForResult gets called in both of these methods. Next up is the createImageFile() method if the user wants to take a picture from the Camera API:

private File createImageFile() throws IOException {
    // Create the Image File name
    String timeStamp = new SimpleDateFormat("yyyyMMdd_HHmmss", Locale.getDefault()).format(new Date());
    String imageFileName = "JPEG_" + timeStamp + "_";

    File storageDir = Environment
            .getExternalStoragePublicDirectory(Environment.DIRECTORY_PICTURES);

    File image = File.createTempFile(
            imageFileName, // Prefix
            ".jpg", // Suffix
            storageDir // Directory
    );

    // Save the file, path for ACTION_VIEW intents
    mCurrentPhotoPath = "file:" + image.getAbsolutePath();
    mUri = Uri.fromFile(image);

    return image;
}

Now finally our startActivityForResult(...) method:

@Override
protected void onActivityResult(int requestCode, int resultCode, Intent data) {
    super.onActivityResult(requestCode, resultCode, data);


    if (requestCode == TAKE_PHOTO_REQUEST && resultCode == RESULT_OK) {
        startUploadProgress();
        showContainer();

        mTypedFile = new TypedFile("image/*", mFile);
        RotatePictureHelper.rotatePicture(mFile, ExampleActivity.this, mAttachment); // Helper class to rotate pictures
        mBus.post(new LoadUploadFileEvent(mTypedFile));

    } else if (requestCode == GET_FROM_GALLERY && resultCode == RESULT_OK) {
        startUploadProgress();
        showContainer();

        mUri = data.getData();
        mTypedFile = UriHelper.handleUri(mUri, this); // Helper class to handle bitmap manipulation
        mFile = mTypedFile.file();
        mBus.post(new LoadUploadFileEvent(mTypedFile));
    } else if (resultCode != Activity.RESULT_CANCELED) {
        Toast.makeText(this, R.string.generalError, Toast.LENGTH_LONG).show();
    }
}

Note that I have already created helper classes to handle bitmap manipulation and picture rotation issues.

STILL, this is very ugly ugly code, and to have this repeated in 5 classes.

I have a few ideas in mind right now:

  • Create a service and pass in needed variables to that service to handle this.
  • Moving AlertDialog options to a helper class and call different AlertDialogs based on instanceOf whatever Activity is calling it.
  • Should I create a parent Activity class that has these methods and then extend the 5 Activity child classes and call these methods?
\$\endgroup\$

2 Answers 2

4
\$\begingroup\$

If it was me, I should go for the third option.

Make an abstract superclass for your Activity where you can put this code into.

If some points of the method difference, you can refactor that part to an abstract method, so you can implement this part custom in each class.

Now, up to a little comments on your code.
Overall, pretty nice code with a lot of comments.
Some of the comments are not needed, while other are sure on their place so you can understand it quickly.

Their are 2 small things what I see and I prefer to see otherwise.

Edit after your answer :

What I see now is that you call the super of onActivityResult and that's good.
The only thing what is bothering me now is that you repeat the case after the super.

I don't know if you can make of CameraActivity an abstract class, if so try the following :

@Override
protected void onActivityResult(int requestCode, int resultCode, Intent data) {
    super.onActivityResult(requestCode, resultCode, data);

    startProgress();

    switch (requestCode) {
        case TAKE_PHOTO_REQUEST:
            if (resultCode == RESULT_OK) {
                mTypedFile = new TypedFile("image/*", mFile);
                doTakePhotoRequest();
            }
            break;
        case GET_FROM_GALLERY:
            if (resultCode == RESULT_OK) {
                mUri = data.getData();
                mTypedFile = UriHelper.handleUri(mUri, this);
                doFromGallery();
            }
            break;
        default:
            stopProgress();
            Toast.makeText(this, R.string.generalError, Toast.LENGTH_LONG).show();
            break;
    }
}

protected abstract void doTakePhotoRequest(); // give params if needed
protected abstract void doFromGallery(); // give params if needed

Like this you just implement these methods and you don't even have to override onActivityResult.

If it's not possible for an abstract class => just implement those methods empty and you can override them later.

\$\endgroup\$
7
  • \$\begingroup\$ Can you explain where I should use switch instead? Also I am writing up an answer right now, if you care to look at it \$\endgroup\$
    – AndyRoid
    Commented Jul 14, 2015 at 5:25
  • \$\begingroup\$ Points to take note of, I created a CameraActivity class to hold the code and it is extending off of another abstract class that holds my EventBus information. \$\endgroup\$
    – AndyRoid
    Commented Jul 14, 2015 at 5:26
  • \$\begingroup\$ private void showFileOptions() contains a switch statement \$\endgroup\$
    – chillworld
    Commented Jul 14, 2015 at 5:27
  • \$\begingroup\$ Oh you are implying I do not have a default case right? Another point to take note I could probably use a switch statement inside onActivityResult() too. \$\endgroup\$
    – AndyRoid
    Commented Jul 14, 2015 at 5:34
  • \$\begingroup\$ Indeed, that's also part of this community, try to make the code better by following some general rules ;) \$\endgroup\$
    – chillworld
    Commented Jul 14, 2015 at 5:36
0
\$\begingroup\$

To solve this, I went with my 3rd option:

I created an activity called CameraActivity and then extended this Activity in my other 5 Activity classes. The code that each of these classes had was 95% similar with maybe 5% differences each respectively.

My CameraActivity would have the following code:

public class CameraActivity extends Activity {
    /*
      Tracking
     */
    private static final int TAKE_PHOTO_REQUEST = 1;
    private static final int GET_FROM_GALLERY = 2;

    private Uri mUri;
    private String mCurrentPhotoPath;
    private File mFile;
    private TypedFile mTypedFile; // For Retrofit

    @Override
    protected void onActivityResult(int requestCode, int resultCode, Intent data) {
        super.onActivityResult(requestCode, resultCode, data);

        startProgress();

        switch (requestCode) {
            case TAKE_PHOTO_REQUEST:
                if (resultCode == RESULT_OK) {
                    mTypedFile = new TypedFile("image/*", mFile);
                }
                break;
            case GET_FROM_GALLERY:
                if (resultCode == RESULT_OK) {
                    mUri = data.getData();
                    mTypedFile = UriHelper.handleUri(mUri, this);
                }
                break;
            default:
                stopProgress();
                Toast.makeText(this, R.string.generalError, Toast.LENGTH_LONG).show();
                break;
        }
    }

    protected void showFileOptions() {
        new AlertDialog.Builder(this)
                .setItems(R.array.uploadOptions, new DialogInterface.OnClickListener() {
                    @Override
                    public void onClick(DialogInterface dialog, int which) {
                        switch (which) {
                            case 0:
                                dispatchTakePicture();
                                break;
                            case 1:
                                dispatchUploadFromGallery();
                                break;
                            default:
                                dispatchUploadFromGallery();
                                break;
                        }
                    }
                })
                .setNegativeButton(android.R.string.cancel, new DialogInterface.OnClickListener() {
                    @Override
                    public void onClick(DialogInterface dialog, int which) {
                        dialog.cancel();
                    }
                })
                .show();
    }

    // Other code for handling Uri and File goes here....

}

Now I can extend this Activity in my other Activity classes and implement the rest of the 5% differences there. Take note that showFileOptions() has changed to protected status so I can call it from a child Activity. As an example here is one activity.

public class PhotoUploadActivity extends CameraActivity {

    // Initialize methods
    @Override
    public void onCreate(Bundle savedInstanceState) {
        super.onCreate(savedInstanceState);
        showFileOptions(); // Calling AlertDialog from Parent Activity
    }

    @Override
    public void onActivityResult(int requestCode, int resultCode, Intent data) {
        // Super call executes code in CameraActivity
        super.onActivityResult(requestCode, resultCode, data);
        switch (requestCode) {
            case TAKE_PHOTO_REQUEST:
                if (resultCode == RESULT_OK) {
                    // Implement logic if user take a photo, do something with mUri or mTypedFile
                }
                break;
            case GET_FROM_GALLERY:
                if (resultCode == RESULT_OK) {
                    // Implement logic if user gets something from gallery, do something with mUri or mTypedFile
                }
                break;
            default:
                break;
        }
    }

}

Now I can simply extend my 5 child Activity classes call super.onActivityResult(...) in those classes and factor in the remainder 5% code while CameraActivity handles the rest.

\$\endgroup\$
3
  • \$\begingroup\$ can CameraActivity be an abstract class? \$\endgroup\$
    – chillworld
    Commented Jul 14, 2015 at 6:08
  • \$\begingroup\$ if I already have another abstract class that is being extended by CameraActivity wouldn't that cause issues? Maybe I should merge both but that abstract class is used by other Activities as well that do not need a Camera utility. \$\endgroup\$
    – AndyRoid
    Commented Jul 14, 2015 at 7:17
  • 1
    \$\begingroup\$ you can extends an abstract class with an abstract class without issues. \$\endgroup\$
    – chillworld
    Commented Jul 14, 2015 at 9:35

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.