1
\$\begingroup\$

What the code does is convert an image to a bitmap without having to create 3 arrays to make the conversion. It basically uses 1/3rd of the memory that it would normally use.

I'm trying to refactor this method to make it shorter, less complex and more readable. So far, I've been unsuccessful in the 4 attempts I've made. Every time I try to extract something it says that there are ambiguous return statements.

private Bitmap decodeBitmapFromFile(File file, boolean preview) {
  byte[] byteArr = new byte[0];
  byte[] buffer = new byte[1024];
  int count = 0;
  int len;
  InputStream inputStream = null;

  try {
    inputStream = new FileInputStream(file);
    while ((len = inputStream.read(buffer)) > -1) {
      if (len != 0) {
        if (count + len > byteArr.length) {
          byte[] newBuf = new byte[(count + len) * 2];
          System.arraycopy(byteArr, 0, newBuf, 0, count);
          byteArr = newBuf;
        }

        System.arraycopy(buffer, 0, byteArr, count, len);
        count += len;
      }
    }

    final BitmapFactory.Options options = new BitmapFactory.Options();
    options.inJustDecodeBounds = true;
    BitmapFactory.decodeByteArray(byteArr, 0, count, options);

    options.inPurgeable = true;
    options.inInputShareable = true;
    options.inJustDecodeBounds = false;
    if (preview) {
      options.inSampleSize = getInSampleSize();
    }
    options.inPreferredConfig = Bitmap.Config.ARGB_8888;

    return BitmapFactory.decodeByteArray(byteArr, 0, count, options);
  } catch (FileNotFoundException e) {
    e.printStackTrace();
  } catch (IOException e) {
    e.printStackTrace();
  } finally {
    try {
      inputStream.close();
    } catch (Exception e) {
    }
  }

  return null;
}
\$\endgroup\$
1
\$\begingroup\$

A couple of thoughts.

  1. Always think about breaking your code into discrete units of work, this will make it more logical, more readable and importantly, more testable. In your case, separate out the reading of the data from the creation of the bitmap.
  2. The ambiguous return will either be because you are modifying multiple local properties (count, len, buffer) or because you return a new Bitmap or null from different points in your code. By needing to use count in your Bitmap creation you make it harder to refactor.
  3. If you are concerned about 0 length returns from inputStream.read() consider adding a handbrake to avoid infinite loops - I have never encountered this in real life though.
  4. Does your code perform better than using ByteArrayOutputStream, something like the below.

Code:

//add throws or try/catch as suits your code.

private byte[] readBitmapData() {
    ByteArrayOutputStream buffer = new ByteArrayOutputStream();
    byte[] inputBuffer = new byte[16384]; //16K default buffer size
    int nRead;

    while ((nRead = is.read(inputBuffer, 0, data.length)) != -1) {
        buffer.write(inputBuffer, 0, nRead);
    }

    return buffer.toByteArray();
}

You can check out the OpenJDK code for ByteArrayOutputStream on docjar. Note their code for grow and ensureCapacity.

\$\endgroup\$
0
\$\begingroup\$

Half of the code deals with reading a file into a byte array, which is a very generic operation. Split the code into two functions, one accepting a file and another accepting a byte array. The former can call the latter.

private static Bitmap decodeBitmap(File file, boolean preview)
         throws IOException {
    // Read file fully into byteArr, then...
    return decodeBitmap(byteArr, preview);
}

private static Bitmap decodeBitmap(byte[] byteArr, boolean preview) {
    BitmapFactory.Options options = new BitmapFactory.Options();
    // etc.
}
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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