A short one
Friday 23 February 2007 @ 2:24 pm
Filed under:

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.

— By Jesper de Jong     PermaLink

6 Responses to “A short one”

  1. Gerard Mulder Says:

    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:

    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:

    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:

    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:

    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:

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

Leave a Reply


Menu


Blog Categories

Browse by Date
February 2007
M T W T F S S
 1234
567891011
12131415161718
19202122232425
262728EC

Upcoming Events

Monthly Archives

Recent Comments

Links


XML Feeds Option


Get Firefox  Powered by WordPress

code validations
Valid RSS 2.0  Valid Atom 0.3
Valid W3C XHTML 1.0  Valid W3C CSS