3
\$\begingroup\$

I needed to convert a formatted file size back into its length in bytes. I came up with the following code but want it reviewed:

/**
 * Converts a formatted size to its equivalent in bytes.
 * 
 * @param size
 *            The formatted file size. Examples: "125.5 MB", "1KiB", "0.8 GB"
 * @return
 * @throws NumberFormatException
 */
public static long getBytesFromFormattedSize(final String size) throws NumberFormatException {
    final String[] arr = size.toUpperCase().split(" ");
    if (arr.length != 2) {
        if (arr.length == 1 && Character.isDigit(size.charAt(0))) {
            // Treat this as a formatted size missing the space.
            return getBytesFromSize(size);
        }
        throw new IllegalArgumentException("Expected '<size> <unit>', got '" + size + "'");
    }
    final char unit = arr[1].charAt(0);
    if (unit == 'B') {
        return Integer.parseInt(arr[0]);
    }
    final int bytes = arr[1].length() == 3 && arr[1].charAt(1) == 'I' ? 1000 : 1024;
    final char[] units = {
            'K', 'M', 'G', 'T', 'P', 'E'
    };
    int exp = 1;
    for (final char ch : units) {
        if (unit == ch) {
            break;
        }
        exp++;
    }
    return (long) (Double.parseDouble(arr[0]) * Math.pow(bytes, exp));
}

private static long getBytesFromSize(final String size) {
    String fixedSize = "";
    for (final char c : size.toCharArray()) {
        if (Character.isLetter(c) && !fixedSize.contains(" ")) {
            fixedSize += ' ';
        }
        fixedSize += c;
    }
    return getBytesFromFormattedSize(fixedSize);
}

Some tests:

String[] sizes = {
        "1 KB", "100 MB", "100MiB", "1 GB", "100B", "1.5 MB"
};
for (String size : sizes) {
    System.out.println(String.format("%s = %d bytes", size, getBytesFromFormattedSize(size)));
}

Output:

1 KB = 1024 bytes
100 MB = 104857600 bytes
100MiB = 100000000 bytes
1 GB = 1073741824 bytes
100B = 100 bytes
1.5 MB = 1572864 bytes
\$\endgroup\$
  • \$\begingroup\$ Do you wish to consider 1KB vs 1Kb? \$\endgroup\$ – thepace Jan 16 '15 at 10:09
  • 1
    \$\begingroup\$ Welcome to CodeReview, hope you get some great answers! \$\endgroup\$ – Abbas Jan 16 '15 at 10:17
3
\$\begingroup\$
  1. The function throws NumberFormatException and IllegalArgumentException.
  2. Your implementation caters to your edge cases, take these for example:

    • 1Kb (Do you wish to handle bits). This gets converted to 1KB (unwantedly).
    • 1M iB (Incorrect position of space)
    • 1 (No units).

    Not sure if the input is manual or system feeded.
    Why not have a function that validates the input and if it can be tailored (1MiB), it returns a operable string.

  3. A Map can be used instead of the units' char array, for loop and Math.pow for each input.
  4. getBytesFromFormattedSize() calls getBytesFromSize() which again calls getBytesFromFormattedSize(). Instead a flow as this would look clean.

     getBytesFromFormattedSize(){
         //Get a valid string (if possible) (in two forms value and unit)
         // Proceed thereof.
     }
    
  5. fixedSize.contains() has an O(n) time complexity. Instead a boolean would easily suffice the otherwise redundant call once we insert a space.
  6. Names such as arr, size, bytes are ignorant to their puprose.
  7. Too much of hard coded values to actually understand the flow.
  8. Should I return 1000? http://en.wikipedia.org/wiki/Mebibyte
  9. return Integer.parseInt(arr[0]); assumes that the value would not be greater than INT_MAX? Does the input guarantee that?
  10. If size is an empty string, charAt(0) could give exception IndexOutOfBoundsException().
\$\endgroup\$
  • \$\begingroup\$ Some good stuff to consider, thanks. I'll wait a bit for any other answers and then mark one as accepted later. \$\endgroup\$ – Jared Rummler Jan 16 '15 at 12:05

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.