Menu ▾ ▴

#329 Backwards calls to Math.min/max for bounds check

3.0.1
closed-fixed
Tagir Valeev
None
5
2015-01-11
2015-01-06
Jesse Glick
No

I have now seen two cases where experienced Java developers wrote something like

int rawInput = …;
int valid = Math.min(0, Math.max(100, rawInput));

This looks right as a casual glance—you are specifying the minimum and maximum values—but in fact always sets valid to 0! They meant to write:

int rawInput = …;
int valid = Math.max(0, Math.min(100, rawInput));

Similarly,

int rawInput = …;
int valid = Math.max(100, Math.min(0, rawInput));

always sets it to 100, and should be corrected to

int rawInput = …;
int valid = Math.min(100, Math.max(0, rawInput));

It seems like it should be straightforward to detect the bad idioms, at least for arguments of type int or long, so long as the bounds are given as constants. There might be subtleties for float- or double-valued calls, so probably the check should just be skipped if either bound is -0 or NaN.

(It would be helpful to have a library function that does the combined bounding operation, yet Math does not have it, nor does Guava nor Apache Commons Lang.)

Discussion

  • Tagir Valeev

    Tagir Valeev - 2015-01-10
    • status: open --> open-accepted
    • assigned_to: Tagir Valeev
     

  • Tagir Valeev

    Tagir Valeev - 2015-01-10

    Interesting idea. Will try to implement.

     

  • Tagir Valeev

    Tagir Valeev - 2015-01-10
    • status: open-accepted --> closed-fixed
    • Group: 3.x --> 3.0.1
     

  • Jesse Glick

    Jesse Glick - 2015-01-11

    Nice.

    For the test, I noticed that all the @NoWarning tests had Math.max wrapped in Math.min. It would be a good idea to also assert @NoWarning on correct cases of Math.min wrapped in Math.max.

    The occurrence you found was one of the two cases that inspired this RFE, from Jenkins PR 1514. (The other also involved Jenkins code, but written after the fork from Hudson, by a different person.)

     

Log in to post a comment.

MongoDB Logo MongoDB