Fun with casts



  • Hi all, found this fun snippet in our companies main framework today:


    int width, height;
    if (biIn.getWidth() > biIn.getHeight())
       {
          width = boxSize;
          height = ( int ) ( ( ( float )width / ( float )biIn.getWidth() ) * ( float )biIn.getHeight());
       }
    else
       {
          height = boxSize;
          width = ( int ) ( ( ( float )height / ( float )biIn.getHeight() ) * ( float )biIn.getWidth());
       }

    boxSize is an int and getHeight() and getWidth() both return ints...



  • Well IF they dont want integer division in there thats not really WTFish.

    The division and multiplication is done in floating point arithmetic and then its converted to int again.
    You could argue that this is very ressource hungry, that it is rarely needed, etc. yadda yadda; BUT IF this is intentional then its not a WTF!
     



  • Well, that was my initial gut reaction too, but he could have avoided any casting by doing the multiplication first (so long as he's sure he can avoid hitting MaxValue):

    Ie. rather than (int)((float)a / (float)b * (float)c)

    you can do: b * c \ a

    Still, at least we can see he thought about it a bit, rather than just going for a / b * c 



  • I looks pretty much like some code I wrote a while ago. I would definitely cast to double instead of float, though, and round the value before casting it back to int.

    It can be done with integer operations also, if you don't care as much about the accuracy, and only if you know that the values are low enough not to cause an overflow.

    Sometimes you will need to do a lot of casting when you are using a strictly typed language. If you wrote it in VB instead, it would look like the code was more straight forward, but it would still be doing all the casting in the background.



  • I went with:


    int width = boxSize;
    int height = boxSize;

    if (biIn.getWidth() > biIn.getHeight())
       {
          height = Math.round(width * biIn.getHeight() / biIn.getWidth());
       }
    else if (biIn.getWidth() < biIn.getHeight())
       {
          width = Math.round(height * biIn.getWidth() / biIn.getHeight());
       }

     which is more like a * c / b. 



  • If this is Java, how is that supposed to work without casting?  Why even bother with the "Math.round" business?  You realize that without casting the multiplication and division will only return integers?  If this code has to do with the aspect ratio of a GUI element, as appears to be the case, then it simply won't be worth a damn without the casts.



  • hehe yer you were right about Math.round(), my bad.  We're only after integer values since the result is used in a html imgs width and height attributes.  ideally the images will already be in the correct dimensions e.g. 200x200, this bit is only really used for when the clients don't follow the image upload guidelines.  obviously we'd all love to say "how likely is that to happen" :¬)



  • Also, it's silly to talk about the performance of casts in such a one-off calculation like this.  I just did a little test, doing an operation with integers 100 million times, and then doing the same operation 100 million times with 4 casts to float on the integers.


    // pure integers
    for (int i = 0; i < 100000000; i++)
    {
    result = (result + i) / denom;
    }

    // cast to float and back to int
    for (int i = 0; i < 100000000; i++)
    {
    result = (int)(((float)result + (float)i) / (float)denom);
    }

    The cast version took three times as long... (9 seconds vs 3) but if it was only 1000 times (and in your case it's only ONE time, probably) the difference would be 0.00006 seconds.  Does that really matter?  Just suck it up and use casts!



  • The casting is a very minor WTF, the WTF if you ask me is that you didn't understand why the casting is necessary. You could write it with less casts, but the uncasted arguments will be implicitly widened to float anyway.

    float fw = biIn.getWidth(), fh = biIn.getHeight();   // only cast them once
    float aspect = fw / fh;
    if(aspect > 1.0f){
        width = boxSize;
        height = (int)(boxSize / aspect);
     } else {
        height = boxSize;
        width = (int)(boxSize * aspect);
    }

    Looks cleaner, but boxSize is still being cast for all those operations with aspect. Yes, it could all be done with integer arithmetic in this case (image dimensions are unlikely to be larger than 2^16), but the code is more readable with the operations done in this order and the performance hit will be negligible. 



  • You could also use BigDecimal, but unless biln was changed to return BigDecimal numbers, the performance hit of creating BigDecimal objects would most likely be worse than that of recasting.

    P.S. teedyay, b * c / a is not equal to a / b * c, but a * c / b is.



  • [quote user="teedyay"]

    Well, that was my initial gut reaction too, but he could have avoided any casting by doing the multiplication first (so long as he's sure he can avoid hitting MaxValue):

    Ie. rather than (int)((float)a / (float)b * (float)c)

    you can do: b * c \ a

    Still, at least we can see he thought about it a bit, rather than just going for a / b * c 

    [/quote]

    That's a good point.  Doing the multiplication before the division may also lower rounding errors, depending on how accurate and deep the interpreter handles division.

    Obviously something like 150160/180 will give the same result as 160/180150, but if we were going to involve numbers on the order of .0000473, it could throw off the results a bit.  Same thing with dividing 12 trillion by 600 before multiplying it by 23.7, for example.

    It's just a good habit to know the numbers you are manipulating and designing the algorithm with them in mind.



  • [quote user="writejustify"][quote user="teedyay"]

    Well, that was my initial gut reaction too, but he could have avoided any casting by doing the multiplication first (so long as he's sure he can avoid hitting MaxValue):

    Ie. rather than (int)((float)a / (float)b * (float)c)

    you can do: b * c \ a

    Still, at least we can see he thought about it a bit, rather than just going for a / b * c 

    [/quote]

    That's a good point.  Doing the multiplication before the division may also lower rounding errors, depending on how accurate and deep the interpreter handles division.

    Obviously something like 150160/180 will give the same result as 160/180150, but if we were going to involve numbers on the order of .0000473, it could throw off the results a bit.  Same thing with dividing 12 trillion by 600 before multiplying it by 23.7, for example.

    [/quote]

    Not in java or any (most?) other C-derived language when doing integer math.



  • [quote user="djork"](and in your case it's only ONE time, probably) the difference would be 0.00006 seconds.  Does that really matter?  Just suck it up and use casts![/quote]

    That snippet comes out of a function that is being used to dynamically resize images pulled out of a db.  I'm talking about every single user uploaded image.  The framework is being developed this week for 4 companies who will share the site.  In Jan it's going to be extended to about 20 companies.  They went for the gimmicky "micro-site" idea someone proposed.  Each company uploads images for their products, services and corp identity.  Instead of using the function at the point of upload it's going to get called each and every time the image is downloaded.  I just figured the casting was a wtf as I assumed it would have quite a big performance penalty when used in that context.  Guess you showed me otherwise :¬)



  • [quote user="Tann San"]

    [quote user="djork"](and in your case it's only ONE time, probably) the difference would be 0.00006 seconds.  Does that really matter?  Just suck it up and use casts![/quote]

    That snippet comes out of a function that is being used to dynamically resize images pulled out of a db.  I'm talking about every single user uploaded image.  The framework is being developed this week for 4 companies who will share the site.  In Jan it's going to be extended to about 20 companies.  They went for the gimmicky "micro-site" idea someone proposed.  Each company uploads images for their products, services and corp identity.  Instead of using the function at the point of upload it's going to get called each and every time the image is downloaded.  I just figured the casting was a wtf as I assumed it would have quite a big performance penalty when used in that context.  Guess you showed me otherwise :¬)

    [/quote]

    That's the sort of system that begs to have some sort of caching built in, to avoid actually resizing the same image repeatedly.  If you're generating a bajillion thumbnails, the multiplication and division to determine the dimensions of each thumbnail is trivial compared to the cost of actually generating the thumbnail.  If you're just calculating the dimensions so that you can tell the browser to resize the full-size image to thumbnail size, then you're abusing bandwidth.

    Best: generate thumbnails at upload
    Second best: generate thumbnails the first time they are needed, and delete them when their source images get re-uploaded
    Worst: generate thumbnails on every access



  • [quote user="Ozru"]
    Best: generate thumbnails at upload
    Second best: generate thumbnails the first time they are needed, and delete them when their source images get re-uploaded
    Worst: generate thumbnails on every access
    [/quote]


    I think the actual method: pulling the images out of a database and resizing with HTML is only slightly better than generating thumbnails on the fly.  It's abusing bandwidth and the database at the same time.  The images should definitely be stored on disk.  Why are people afraid of files and folders nowadays?  It's like the DB is the only place suitable for storing data...

    Storing files on disk allows the web server and OS to cache and serve up data far more efficiently (and at a level closer to the user) than the web app and DB engine would.



  • My suggestions where (1) resize the image when it's first uploaded (2) do 1 but also store it at it's original dimensions (3) do 2 but store it at multiple dimensions i.e. a thumbnail size, med size and large.  They store everything in the db because they haven't set up the dual file path system where most things are contained in the one java file and then other things are served from their own folders.  In the end it all came down to time, money and insane deadlines.


Log in to reply