Here are a few lines of code I found in an application that I’m currently maintaining. I don’t know who wrote this code originally.


// Values read from somewhere
int budgetSpent = ...;
int budget = ...;

// Compute percentage of budget spent
int percentage = Math.round(budgetSpent * 100 / budget);


The last line of code is wrong. Can you tell me why? Every Sun Certified Java Programmer should be able to anwer this.

6 Responses to “A short one”

1. Gerard Mulder Says:
   February 27th, 2007 at 2:29 pm

   If there’s no budget at all (let’s say: 0), Mr. D.B. Zero of Fatal Exception Office comes around the corner

2. Jesper de Jong Says:
   February 28th, 2007 at 3:12 pm

   Ok Gerard, that’s one, but there’s something else too.
   I’m not going to give a hint yet because that might give it away…

3. Willem Spruijt Says:
   March 4th, 2007 at 4:39 pm

   I think its a division/typecasting problem. Since budgetSpent and budget are both declared as int’s, when dividing them, the result of the division is an int. The problem is that this integer is not rounded but it’s just the number “without the precision” (so 7/2 would be 3 instead of 4). The correct code would be:

   int percentage = Math.round( (float) budgetSpent * 100 / (float) budget);

   OR, this is maybe more clear (and as we’re taught at school ):

   int percentage = Math.round(( (float) budgetSpent / (float) budget) * 100);

   The division reults in a float, and when rounding this float with Math.round([float]) returns an integer.

   More of this problem is described here.
   -Willem

4. Jesper de Jong Says:
   March 5th, 2007 at 12:22 pm

   You’re right, Willem.

   The variables ‘budgetSpent’ and ‘budget’ are both ints, and the value 100 in the expression is an integer literal. So the expression ‘budgetSpent * 100 / budget’ is computed with integer arithmetic and the result is an integer.

   The result of the expression is passed to Math.round(…). The result of the expression is implicitly converted from int to float (because the version of Math.round(…) that takes a float is called). So, calling Math.round(…) does not do anything here. You pass in an int, and you always get back exactly the same value as you put in.

   To make it work as intended, you should make sure that the expression is done with floating point arithmetic. To do this it’s enough to make one of the operands a float (or a double), for example:

   int percentage = Math.round(budgetSpent * 100f / budget);

   (Note the “f” which makes it a float literal). I just realized I could have said “add one letter to the expression to make it correct”…

5. Willem Spruijt Says:
   March 6th, 2007 at 12:39 am

   Hi Jesper,

   I agree with you that this is a realy tricky problem which is hard to detect, because the the deviation is only 1 percent.

   Keep the tricky problems comin’!

   -Willem

6. Anonymous coward Says:
   April 17th, 2007 at 10:13 pm

   Of course this problem would be trivial to detect with some unit tests.

Leave a Reply