7
\$\begingroup\$

The method length() in File returns the length of a file in Bytes, this code transforms that into b/kb/mb as can be seen in the Windows Explorer

I ended up with the code below, let me know if it can be optimized or the logic can be improved.

public static String getFileSize(File file) {
    String modifiedFileSize = null;
    double fileSize = 0.0;
    if (file.isFile()) {
        fileSize = (double) file.length();//in Bytes

        if (fileSize < 1024) {
            modifiedFileSize = String.valueOf(fileSize).concat("B");
        } else if (fileSize > 1024 && fileSize < (1024 * 1024)) {
            modifiedFileSize = String.valueOf(Math.round((fileSize / 1024 * 100.0)) / 100.0).concat("KB");
        } else {
            modifiedFileSize = String.valueOf(Math.round((fileSize / (1024 * 1204) * 100.0)) / 100.0).concat("MB");
        }
    } else {
        modifiedFileSize = "Unknown";
    }

    return modifiedFileSize;
}
\$\endgroup\$
  • \$\begingroup\$ You might find this interesting- stackoverflow.com/questions/3758606/…. \$\endgroup\$ – abksrv Jul 23 '15 at 15:40
  • \$\begingroup\$ Your output is incorrect. 1 MB = 1000*1000 bytes. 1 MiB = 1024*1024 bytes. \$\endgroup\$ – CodesInChaos Jul 23 '15 at 16:19
  • \$\begingroup\$ i don't know about that here MB = 1204 * 1024 Bytes \$\endgroup\$ – mak Jul 23 '15 at 16:23
7
\$\begingroup\$

Logic

You can simplify your logic by testing for the biggest known extension, for example

if (fileSize > 1024 * 1024)
    doMiB();
} else if (fileSize > 1024) {
    doKiB();
} else {
    doByte();
}

Rounding

Don't use Math.round(x / 100.0) * 100. for rounding to two decimal places use the appropriate library function

Argument checking

Don't use default values ("Unknown") for invalid input. If the input to your getFileSize method is not a file, throw an exception

if (!file.isFile()) {
    throw new IllegalArgumentException("Expected argument to be a file");
}

Fast return

This is more personal taste but I prefer to return as soon as possible. If you know the result will be "100.31 KiB" why save it into a local variable instead of returning it immediately?

Conclusion

I would improve the code as followed

private static final DecimalFormat format = new DecimalFormat("#.##");
private static final long MiB = 1024 * 1024;
private static final long KiB = 1024;

public String getFileSize(File file) {

    if (!file.isFile()) {
        throw new IllegalArgumentException("Expected a file");
    }
    final double length = file.length();

    if (length > MiB) {
        return format.format(length / MiB) + " MiB";
    }
    if (length > KiB) {
        return format.format(length / KiB) + " KiB";
    }
    return format.format(length) + " B";
}
| improve this answer | |
\$\endgroup\$
  • \$\begingroup\$ @EmilyL. i am actually now in favor of Using DecimalFormat \$\endgroup\$ – mak Jul 23 '15 at 14:15
  • \$\begingroup\$ Thank you @EmilyL. yes OP should use DecimalFormat I will update the link \$\endgroup\$ – ooxi Jul 23 '15 at 14:43
  • \$\begingroup\$ In addition to these, as a scalable and cleaner way you can make an enum of Units with each having method fromBytes() to convert bytes to target unit. \$\endgroup\$ – abksrv Jul 23 '15 at 15:45
  • 1
    \$\begingroup\$ There should be a semi-colon after return format.format(length) + " B" I suggested an edit and but it is rejected twice \$\endgroup\$ – Amar Ilindra May 3 '18 at 14:55
  • \$\begingroup\$ @AmarIlindra you are correct, an ; was missing. I fixed this mistake, don't know why your revisions where rejected \$\endgroup\$ – ooxi Feb 17 '19 at 19:51
6
\$\begingroup\$

Use helper functions

The logic performed by your math isn't immediately obvious at a glance. You should introduce a helper function:

static double roundToDecimals(double aValue, int aDecimals){
    ...
}

Even better, just use DecimalFormat:

DecimalFormat df = new DecimalFormat("#.##");
String output = df.format(1.2345678);  // "1.23"

Work in logarithms

You can simplify your determination of the correct suffix by using the logarithm of the file size.

First of note that \$1024 = 2^{10}\$, \$1024*1024 = 2^{20}\$ and so forth. You're checking if \$x < 2^{10}\$ and then if \$x < 2^{20}\$ etc.

It's much easier to take the two-logarithm of \$x\$ and then check \$\log_2(x) < 10\$, \$\log_2(x) < 20\$ etc...

Now if you take \$k=floor(\log_2(x) / 10)\$ you will end up with \$k=0\$ for bytes, \$k=1\$ for KiB, \$k=2\$ for MiB etc. Which means that you can simply index into an array of the suffixes to get the right suffix, then upscale the result to get the correct number of digits.

Like this:

public static double log2(long n){
    // Implement this but without inaccuracies due to FP math. 
    // Just count the number of leading zeros and do the math.
    return (Math.log(n) / Math.log(2));
}

public static String getFileSize(File file) {
    ...
    long logSize = (long)log2(fileSize);
    final String[] suffixes = new String[]{" B", " KiB", " MiB", " GiB", " TiB", " PiB", " EiB", " ZiB", " YiB"};

    int suffixIndex = (int) (logSize / 10); // 2^10 = 1024

    double displaySize = fileSize / Math.pow(2, suffixIndex*10);
    DecimalFormat df = new DecimalFormat("#.##");
    return df.format(displaySize) + suffixes[suffixIndex];

Other comments

You should keep your fileSize in long format for accuracy. Also you can move the fileSize variable into the scope of the if where it is initialized and used.

| improve this answer | |
\$\endgroup\$
  • \$\begingroup\$ @Emily L i need clarification on this displaySize = df.format(displaySize); Since the displaySizeis type of double and df.format(...) returns String??? \$\endgroup\$ – mak Jul 23 '15 at 15:00
  • \$\begingroup\$ @EmilyL. exactly, but the confusion is here displaySize = df.format(displaySize); \$\endgroup\$ – mak Jul 23 '15 at 15:16
  • \$\begingroup\$ you are assigning String to double??? and one more thing , What if the File size ==0 ?? i just try a simple Test and the output was this File Size = 0 Log Size = -9223372036854775808 suffix Index = 858993460 which also caused java.lang.ArrayIndexOutOfBoundsException Because the the suffix index was = 85899........ \$\endgroup\$ – mak Jul 23 '15 at 15:31
  • \$\begingroup\$ @mak By definition \$log(0)=-\infty\$. Just handle the case if fileSize = 0 specially. \$\endgroup\$ – Emily L. Jul 23 '15 at 15:42
  • 3
    \$\begingroup\$ @mak I'm not your contractor. Try it and see if it works. \$\endgroup\$ – Emily L. Jul 23 '15 at 16:50

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.