4
\$\begingroup\$

I could really use another set of eyes on this. No one else in the office has a lot of experience working with Android, so I'm on my own with this one.

The goal of this class is to encapsulate an image which can be sourced from a camera or file so that it can be uploaded to one of our servers using a POST request. Just looking at the code, it feels overly-complicated, and I'm afraid that this is super inefficient.

package com.package.android; import android.content.Context; import android.database.Cursor; import android.graphics.Bitmap; import android.graphics.BitmapFactory; import android.net.Uri; import android.os.Build; import android.os.Environment; import android.os.Parcel; import android.os.Parcelable; import android.provider.MediaStore; import android.util.Log; import java.io.ByteArrayInputStream; import java.io.ByteArrayOutputStream; import java.io.File; import java.io.FileInputStream; import java.io.FileNotFoundException; import java.io.IOException; import java.io.InputStream; import java.text.SimpleDateFormat; import java.util.Date; import com.package.R; class UploadableImage implements Parcelable { public static final String UPLOADABLE_IMAGE = "UPLOADABLE_IMAGE"; public static final Parcelable.Creator<UploadableImage> CREATOR = new Parcelable.Creator<UploadableImage>() { public UploadableImage createFromParcel(Parcel in) { return new UploadableImage(in); } public UploadableImage[] newArray(int size) { return new UploadableImage[size]; } }; private static final String JPEG_FILE_PREFIX = "IMG_"; private static final String JPEG_FILE_SUFFIX = ".jpg"; private final String TAG = "UploadableImage"; private File file; private String filepath; private Uri uri; private Bitmap bmp; private Context context; private AlbumStorageDirFactory mAlbumStorageDirFactory = null; private int fileSize; private String descriptionText; private String nameText; private String emailText; public UploadableImage(Context context) { this.context = context; file = null; filepath = null; uri = null; bmp = null; fileSize = -1; descriptionText = null; nameText = null; emailText = null; setupStorageDirFactory(); } public UploadableImage(Parcel in) { readFromParcel(in); setupStorageDirFactory(); } static String filePathFromUri(Context context, Uri uri) { if (uri == null) { return null; } //see http://stackoverflow.com/a/16511111/940217 String filepath = null; String[] projection = {MediaStore.MediaColumns.DATA}; Context ctx = context.getApplicationContext(); if (ctx != null) { Cursor cursor = ctx.getContentResolver().query(uri, projection, null, null, null); if (cursor != null) { // Here you will get a NPE if cursor is null. // This can be if you used OI File Manager (or other 3rd party) for picking the media cursor.moveToFirst(); int columnIndex = cursor.getColumnIndexOrThrow(MediaStore.MediaColumns.DATA); filepath = cursor.getString(columnIndex); cursor.close(); } } else { filepath = uri.getPath(); // For OI/Astro/Dropbox, etc. } return filepath; } private void setupStorageDirFactory() { if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.FROYO) { mAlbumStorageDirFactory = new FroyoAlbumDirFactory(); } else { mAlbumStorageDirFactory = new BaseAlbumDirFactory(); } } public void setContext(Context context) { this.context = context; } public boolean valid() { return (context != null && this.getFilepath() != null); } Uri getUri() { if (this.uri != null) { return this.uri; } if (this.file != null) { this.uri = Uri.fromFile(this.file); return this.uri; } if (this.bmp == null || context == null) { return null; } if (this.filepath != null) { this.uri = Uri.fromFile(new File(this.filepath)); return this.uri; } ByteArrayOutputStream bytes = new ByteArrayOutputStream(); this.bmp.compress(Bitmap.CompressFormat.JPEG, 100, bytes); Context ctx = context.getApplicationContext(); if (ctx == null) { return uri; } String path = MediaStore.Images.Media.insertImage(ctx.getContentResolver(), this.bmp, "Title", null); this.uri = Uri.parse(path); return uri; } public void setUri(Uri uri) { Log.d(TAG, "Setting URI to " + uri.toString()); this.uri = uri; } private void setBitmapFromUri(Uri uri) { Uri tempUri = uri; Bitmap tempBmp = null; if (this.uri != null && uri == null) { tempUri = this.uri; } try { tempBmp = MediaStore.Images.Media.getBitmap(context.getContentResolver(), tempUri); } catch (IOException e) { Log.e(TAG, "IOException: tried getting the bitmap from the MediaStore using the URI and failed. The URI given was: " + tempUri.toString()); e.printStackTrace(); } this.bmp = tempBmp; } public File createFile() throws IOException { String timeStamp = new SimpleDateFormat("yyyyMMdd_HHmmss").format(new Date()); String imageFileName = JPEG_FILE_PREFIX + timeStamp + "_"; File albumF = getAlbumDir(); this.file = File.createTempFile(imageFileName, JPEG_FILE_SUFFIX, albumF); return this.file; } public boolean deleteFile() { return this.file.delete(); } /* Photo album for this application */ private String getAlbumName() { return context.getString(R.string.album_name); } private File getAlbumDir() { File storageDir = null; if (Environment.MEDIA_MOUNTED.equals(Environment.getExternalStorageState())) { storageDir = mAlbumStorageDirFactory.getAlbumStorageDir(getAlbumName()); if (storageDir != null) { if (!storageDir.mkdirs()) { if (!storageDir.exists()) { Log.w(TAG, "failed to create directory"); return null; } } } } else { Log.w(TAG, "External storage is not mounted READ/WRITE."); } return storageDir; } public File getFile() { return file; } public void setFile(File file) { this.file = file; this.filepath = this.file.getAbsolutePath(); } public Bitmap getBmp() { if (this.bmp != null) { return this.bmp; } Bitmap resultBitmap = null; Uri tempUri = this.getUri(); if (tempUri != null) { setBitmapFromUri(tempUri); } if (this.bmp != null) { resultBitmap = this.bmp; } this.bmp = resultBitmap; return resultBitmap; } public void setBmp(Bitmap bmp) { this.bmp = bmp; } private byte[] getBmpAsByteArray(Bitmap data) { if (data == null) { return null; } ByteArrayOutputStream bos = new ByteArrayOutputStream(); data.compress(Bitmap.CompressFormat.JPEG, 100 /*ignored for PNG*/, bos); return bos.toByteArray(); } private void setBitmapFromByteArray(byte[] data) { this.bmp = BitmapFactory.decodeByteArray(data, 0, data.length); } public void updateFilepath() { if (this.uri != null) { setFilepath(filePathFromUri(this.context, uri)); } getFilepath(null); } void invalidate() { if (getFileSize() == 0) { deleteFile(); } file = null; bmp = null; filepath = null; fileSize = -1; uri = null; } String getFilepath() { return getFilepath(null); } public void setFilepath(String filepath) { this.filepath = filepath; } String getFilepath(Uri uri) { if (this.filepath != null) { return this.filepath; } if (this.file != null) { return this.file.getAbsolutePath(); } if (uri == null) { uri = getUri(); } return filePathFromUri(this.context, uri); } public Bitmap getScaledBitmap(int width, int height) { Bitmap scaledBmp = null; InputStream is; Bitmap oBmp = this.getBmp(); if (oBmp != null) { Log.d(TAG, "scaled bitmap was able to use a bitmap"); return Bitmap.createScaledBitmap(oBmp, width, height, false); } Log.d(TAG, "scaled bitmap was not able to use a bitmap and will try to get it from a stream."); try { is = getInputStream(); /* Get the size of the image */ BitmapFactory.Options bmOptions = new BitmapFactory.Options(); bmOptions.inJustDecodeBounds = true; BitmapFactory.decodeStream(is, null, bmOptions); int photoW = bmOptions.outWidth; int photoH = bmOptions.outHeight; is.close(); is = getInputStream(); /* Figure out which way needs to be reduced less */ int scaleFactor = 1; if ((width > 0) || (height > 0)) { scaleFactor = Math.min(photoW / width, photoH / height); } /* Set bitmap options to scale the image decode target */ bmOptions.inJustDecodeBounds = false; bmOptions.inSampleSize = scaleFactor; bmOptions.inPurgeable = true; /* Decode the JPEG file into a Bitmap */ scaledBmp = BitmapFactory.decodeStream(is, null, bmOptions); is.close(); } catch (FileNotFoundException e) { Log.e(TAG, "Could not resize the bitmap - File Not Found Exception"); e.printStackTrace(); } catch (IOException e) { Log.e(TAG, "IO Exception while trying to resize the bitmap"); e.printStackTrace(); } return scaledBmp; } protected int sizeOf(Bitmap data) { /* if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.KITKAT) { return data.getAllocationByteCount(); } else if (Build.VERSION.SDK_INT < Build.VERSION_CODES.HONEYCOMB_MR1) { return data.getRowBytes() * data.getHeight(); } else { return data.getByteCount(); } // NONE OF THE ABOVE RETURN ACCURATE RESULTS! // A Bitmap, when stored as a file takes up more room because it represents // full pixel data and is not compressed on disk. */ byte[] bitmapdata = getBmpAsByteArray(data); return (bitmapdata == null) ? 0 : bitmapdata.length; } public InputStream getInputStream() { InputStream is = null; Bitmap bmp = this.getBmp(); if (this.file == null || this.bmp == null) { loadResources(); } if (bmp != null) { Log.i(TAG, "getting input stream from bitmap"); byte[] bitmapdata = getBmpAsByteArray(bmp); this.fileSize = bitmapdata.length; is = new ByteArrayInputStream(bitmapdata); } else { Log.i(TAG, "getting input stream from file"); try { is = new FileInputStream(this.file); } catch (FileNotFoundException e) { Log.e(TAG, "could not get an input stream from file with path: " + getFilepath()); e.printStackTrace(); } } return is; } private void loadResources() { Uri tempUri = getUri(); if (tempUri != null) { this.file = new File(tempUri.getPath()); } try { this.bmp = MediaStore.Images.Media.getBitmap(context.getContentResolver(), tempUri); } catch (IOException e) { Log.e(TAG, "IOException when trying to load the bitmap from URI: " + tempUri); } } public long getFileSize() { if (this.fileSize != -1) { return this.fileSize; } this.fileSize = sizeOf(getBmp()); return this.fileSize; } public String getDescriptionText() { return descriptionText; } public void setDescriptionText(String descriptionText) { this.descriptionText = descriptionText; } public String getNameText() { return nameText; } public void setNameText(String nameText) { this.nameText = nameText; } public String getEmailText() { return emailText; } public void setEmailText(String emailText) { this.emailText = emailText; } @Override public int describeContents() { return 0; } @Override public void writeToParcel(Parcel dest, int flags) { dest.writeString(filepath); dest.writeString(uri.toString()); dest.writeInt(fileSize); if (fileSize >= 0) { dest.writeByteArray(getBmpAsByteArray(getBmp())); } dest.writeString(descriptionText); dest.writeString(nameText); dest.writeString(emailText); } private void readFromParcel(Parcel in) { file = null; filepath = in.readString(); uri = Uri.parse(in.readString()); fileSize = in.readInt(); if (fileSize >= 0) { byte[] bmpData = new byte[fileSize]; in.readByteArray(bmpData); setBitmapFromByteArray(bmpData); } descriptionText = in.readString(); nameText = in.readString(); emailText = in.readString(); } }
\$\endgroup\$

1 Answer 1

Reset to default
2
\$\begingroup\$ 
static String filePathFromUri(Context context, Uri uri) { Uri getUri() { void invalidate() { String getFilepath(Uri uri) {

Missing access modifier. Is that intentional?

 if (cursor != null) { // Here you will get a NPE if cursor is null. // This can be if you used OI File Manager (or other 3rd party) for picking the media

But you just did an explicit check for null! Did I miss something, or is this comment contradicting the code? Comments that contradict the code represent a conflict, and conflicts should be resolved. Never just delete the comment; carefully check whether the code or the comment is right.

 Bitmap scaledBmp = null; InputStream is; Bitmap oBmp = this.getBmp(); if (oBmp != null) { Log.d(TAG, "scaled bitmap was able to use a bitmap"); return Bitmap.createScaledBitmap(oBmp, width, height, false); }

Don't declare variables if you don't need them. Move the declarations of scaledBmp and is below that if statement. Try to keep all your variable declarations close to where you need them.

A lot of your methods have side effects. I recommend adding javadoc comment blocks to list these side effects, or to rename them to explain that they have side effects. For instance:

private void setBitmapFromUri(Uri uri)

This method sets the bitmap to a bitmap pointed to by the supplied Uri... or, in the event no Uri is supplied, it sets the bitmap based on the current Uri! That's undocumented behavior.

\$\endgroup\$

You must log in to answer this question.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.