Hacker News new | past | comments | ask | show | jobs | submit login

The simple, readable, loop-based snippet mentioned in the article also works for the edge cases. Sometimes it's better to not be so clever.



No, it doesn't. It has the exact same rounding bug described in the article.

The article even mentions:

> FWIW, all 22 answers posted, including the ones using Apache Commons and Android libraries, had this bug (or a variation of it) at the time of writing.


That seems a bit confusing given how the loop works.

999,999 shouldn't be big enough to be affected by floating point rounding.

If the comment is correct then Java will evaluate ( 999999 >= 1000000 ) as true, which simply cannot be correct.


You're misunderstanding the bug.

In the end, the number will be displayed to one decimal place, eg. "1.1 MB" or similar due to the "%.1f" format specifier.

If the input is 999999 bytes, the loop will see that it is less than 1 MB and so will format the number 999.999 into "%.1f kB". When this number is rounded to one decimal place as part of that formatting, it rounds up to "1000.0 kB". This is the wrong output.

There's a bit of a catch 22 because ideally, you would be able to do the rounding first, and then see what units need applying, but you can't do the rounding until you know what the divisor will be.

The article solves this by first manually determining what the cut-off should be (it will be some number like "...999500...") but personally I'd probably just decide to round to significant figures instead so that you can cleanly separate the "rounding" and "unit selection" steps.


Oh, that makes sense, although I'm not sure "1000.0 kB" is strictly wrong in this case.

With the loop at least it's easy to adjust the thresholds if that is desirable, although a comment will be necessary to explain why you are making the cutoffs in weird places.

Interestingly enough in the past I've used a loop similar to this:

  static char suffix[] = { ' ', 'k', 'm', 'g', 't' };
  magnitude = 0;
  while ( value > 1000 )
  {
    value /= 1000;
    magnitude++;
  }
  printf("%.1f%cB", value, suffix[magnitude]);
Which is bad because it's subject to repeated rounding from the division, but avoids the problem you described.


I don't think the multiple divisions will do any harm as you only need 4 digits of precision left at the end.


And the pow and log methods they call run loops anyway. What is wrong with loops anyway?


> the pow and log methods they call run loops anyway.

That's actually not true (as somebody mentioned on reddit) https://github.com/openjdk-mirror/jdk/blob/jdk8u/jdk8u/maste...

It is probably slower anyway, but I was surprised to see no loops


They kind of cheated because they used a hard-coded polynomial approximation of log(x). So there is an implicit unrolled loop going over A_i*x^i.




Guidelines | FAQ | Lists | API | Security | Legal | Apply to YC | Contact

Search: