Commons IO
  1. Commons IO
  2. IO-226

Rounding issue with byteCountToDisplaySize(long size)

    Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 2.0
    • Component/s: Utilities
    • Labels:
      None

      Description

      I do not understand the byteCountToDisplaySize(long size) method which is in class FileUtils of the package org.apache.commons.io.
      If the parameter size is 2047 , the method will return 1 KB.Why it will lose precision.
      I read the code.
      Maybe it is a bug?

        Activity

        Hide
        Sebb added a comment -

        The size is currently rounded down, so it can lose precision.

        However, even if it were rounded to the nearest boundary it would still lose some precision.

        In order to avoid loss of precision it would have to display the value 2047 as 1.9990234375KB which is not very readable.

        For the time being, I have updated the Javadoc to note that the value is rounded down:

        URL: http://svn.apache.org/viewvc?rev=919253&view=rev
        Log:
        IO-226 - document rounding behaviour

        Show
        Sebb added a comment - The size is currently rounded down, so it can lose precision. However, even if it were rounded to the nearest boundary it would still lose some precision. In order to avoid loss of precision it would have to display the value 2047 as 1.9990234375KB which is not very readable. For the time being, I have updated the Javadoc to note that the value is rounded down: URL: http://svn.apache.org/viewvc?rev=919253&view=rev Log: IO-226 - document rounding behaviour
        Hide
        Sebb added a comment -

        Not really a bug anymore

        Show
        Sebb added a comment - Not really a bug anymore
        Hide
        Yury Kats added a comment - - edited

        What is the reason for rounding down?

        it would have to display the value 2047 as 1.9990234375KB which is not very readable.

        This can easily be turned into "1.9" (or "1.99"), which is readable and way more accurate.

        Because of the current behaviour, any project I've been on ends up writing its own byteCountToDisplaySize method and not using FileUtils.
        Who needs a method that returns "1 GB" for a 1.99GB file?

        Show
        Yury Kats added a comment - - edited What is the reason for rounding down? it would have to display the value 2047 as 1.9990234375KB which is not very readable. This can easily be turned into "1.9" (or "1.99"), which is readable and way more accurate. Because of the current behaviour, any project I've been on ends up writing its own byteCountToDisplaySize method and not using FileUtils. Who needs a method that returns "1 GB" for a 1.99GB file?
        Hide
        Mark Miller added a comment -

        Who needs a method that returns "1 GB" for a 1.99GB file?

        +1 - I'm sure everyone just re-implements this method. I have done it and seen it done.

        Show
        Mark Miller added a comment - Who needs a method that returns "1 GB" for a 1.99GB file? +1 - I'm sure everyone just re-implements this method. I have done it and seen it done.
        Hide
        Gary Gregory added a comment -

        +1 here too.

        Would using 2 digit precision all the time be better?

        Show
        Gary Gregory added a comment - +1 here too. Would using 2 digit precision all the time be better?
        Hide
        Alexis Bataille added a comment -

        +1

        Indeed I re-implements this method with :

        
                String displaySize;
        
                if ( size / FileUtils.ONE_GB > 0 )
                {
                    displaySize = String.valueOf( new BigDecimal( size ).divide( BD_ONE_GO, BigDecimal.ROUND_CEILING ) )
                            + " GO";
                }
                else if ( size / FileUtils.ONE_MB > 0 )
                {
                    displaySize = String.valueOf( new BigDecimal( size ).divide( BD_ONE_MO, BigDecimal.ROUND_CEILING ) )
                            + " MO";
                }
                else if ( size / FileUtils.ONE_KB > 0 )
                {
                    displaySize = String.valueOf( new BigDecimal( size ).divide( BD_ONE_KO, BigDecimal.ROUND_CEILING ) )
                            + " KO";
                }
                else
                {
                    displaySize = String.valueOf( size ) + " octets";
                }
                return displaySize;
        
        Show
        Alexis Bataille added a comment - +1 Indeed I re-implements this method with : String displaySize; if ( size / FileUtils.ONE_GB > 0 ) { displaySize = String .valueOf( new BigDecimal( size ).divide( BD_ONE_GO, BigDecimal.ROUND_CEILING ) ) + " GO" ; } else if ( size / FileUtils.ONE_MB > 0 ) { displaySize = String .valueOf( new BigDecimal( size ).divide( BD_ONE_MO, BigDecimal.ROUND_CEILING ) ) + " MO" ; } else if ( size / FileUtils.ONE_KB > 0 ) { displaySize = String .valueOf( new BigDecimal( size ).divide( BD_ONE_KO, BigDecimal.ROUND_CEILING ) ) + " KO" ; } else { displaySize = String .valueOf( size ) + " octets" ; } return displaySize;
        Hide
        Gary Gregory added a comment -

        Feel free to submit a patch to trunk

        Show
        Gary Gregory added a comment - Feel free to submit a patch to trunk
        Hide
        Brice McIver added a comment -

        This patch changes the byteCountToDisplaySize method so it will display at least three significant figures of the calculated value. This patch also includes updated unit tests with the new expected values for some of the calculations (ex. 1.99 GB instead of 1 GB).
        Also added support for YB and ZB bytes sizes when using the BigInteger version of the method.

        Show
        Brice McIver added a comment - This patch changes the byteCountToDisplaySize method so it will display at least three significant figures of the calculated value. This patch also includes updated unit tests with the new expected values for some of the calculations (ex. 1.99 GB instead of 1 GB). Also added support for YB and ZB bytes sizes when using the BigInteger version of the method.
        Hide
        Simon Broadley added a comment -

        The comment here seems to be saying that it has now been changed to show three significant figures, but the version I'm using, from commons-io 2.4 still rounds down?
        Have I misunderstood, is the version information wrong, or has it been rolled back (I hope not).
        Thanks

        Show
        Simon Broadley added a comment - The comment here seems to be saying that it has now been changed to show three significant figures, but the version I'm using, from commons-io 2.4 still rounds down? Have I misunderstood, is the version information wrong, or has it been rolled back (I hope not). Thanks
        Hide
        Mark added a comment -

        Rounding is one issue, but the inconsistent precision is another one. Showing "923 bytes" and then just "1 KB" is not ok. It should rather say "round to N chars", ie. "1.1 KB".

        Show
        Mark added a comment - Rounding is one issue, but the inconsistent precision is another one. Showing "923 bytes" and then just "1 KB" is not ok. It should rather say "round to N chars", ie. "1.1 KB".
        Hide
        Mark added a comment -

        Create new issue IO-373 because this one is closed.

        Show
        Mark added a comment - Create new issue IO-373 because this one is closed.

          People

          • Assignee:
            Unassigned
            Reporter:
            shu kai yuan
          • Votes:
            0 Vote for this issue
            Watchers:
            4 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development