HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavaMinor

Parcelable Bitmap with the intention of uploading the bitmap

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
theuploadingwithintentionbitmapparcelable

Problem

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 CREATOR
= new Parcelable.Creator() {
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 Stri

Solution

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.

Code Snippets

static String filePathFromUri(Context context, Uri uri) {
Uri getUri() {
void invalidate() {
String getFilepath(Uri uri) {
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
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);
    }
private void setBitmapFromUri(Uri uri)

Context

StackExchange Code Review Q#52271, answer score: 2

Revisions (0)

No revisions yet.