While (true) try catch index++



  •  I just found this gem in some of my coworker's code:

    private int assertOrderNbr(int skipOrderNbr, int index, int orderNbr) {

    while (true) {
    try {
    assertEquals("ordernumber is wrong.", index+1, orderNbr);
    return index;
    } catch (AssertionFailedError e){
    if (skipOrderNbr > 0) {
    index++;
    } else {
    fail("ordernumber is wrong.");
    }
    }
    }

    }


    Need I say much?



  •  

    <font size="1">Image credits: cerulean @Typophile</font>



  •  Wow, that guy is incredible!  He has found a way to combine error checking, exception handling AND unit testing all in one!  You should be so blessed to be gazing upon his code.



  • @TheRider said:

    Need I say much?

    We share your pain. What went on in his head? Was an if-statement too difficult? Not creative enough? Was mezcalin involved? Or is this an example of "programming by IDE", where you type a statement and if that doesn't work you just select options from the IDE's suggestions until it does?



  • Ahh, I see. A great, useful piece of code. Should be refactored though:

    private int assertOrderNbr(int skipOrderNbr, int index, int orderNbr) {
      if (skipOrderNbr > 0) {
        if (index >= orderNbr) {
          //Simulate waiting time for integer overflow so we don't break stuff
          sleep(100);
        }
        index = orderNbr - 1;
      }
      if (index == orderNbr - 1) {
        return index
      } else {
        fail("ordernumber is wrong.");
      }
    }


  • @derula said:

    Ahh, I see. A great, useful piece of code. Should be refactored though:

    Not like that, do it like this:

    private int assertOrderNbr(int skipOrderNbr, int index, int orderNbr) {
    if (skipOrderNbr <= 0 && index != orderNbr - 1) {
    fail("ordernumber is wrong.");
    } else {
    return orderNbr -1;
    }
    }


  • @Gnonthgol said:

    @derula said:

    Ahh, I see. A great, useful piece of code. Should be refactored though:

    Not like that, do it like this:

    private int assertOrderNbr(int skipOrderNbr, int index, int orderNbr) {
    if (skipOrderNbr <= 0 && index != orderNbr - 1) {
    fail("ordernumber is wrong.");
    } else {
    return orderNbr -1;
    }
    }

    You didn't get derula's real joke - the part to simulate integer overflow.



  • @Gnonthgol said:

    Not like that, do it like this:

    Or this (if the use of assertions is legitimate here):

    private int assertOrderNbr(int skipOrderNbr, int index, int orderNbr) {
      assertTrue("ordernumber is wrong.", skipOrderNbr > 0 || index == orderNbr - 1);
      return orderNbr - 1;
    }

    Also: What was he thinking? Can you give an example of how this is called? Or any comments about it?



  •  @derula said:

    Also: What was he thinking? Can you give an example of how this is called? Or any comments about it?

    Well, this happens in the context of a JUnit-Test, where there is a file with a few rows of data, and this data needs to be validated row by row and the valid rows need to be inserted into a database table. Invalid rows would, therefore, not be inserted in the database. Such as this:

    [code]1 25 this is maybe a valid row
    2 X this is maybe a faulty row
    3 12 another valid row[/code]

    In this particular case, the numbers 1, 2 and 3 are order numbers, and the order number 2 would be missing in the database because the corresponding row does not validate.

    [code]assertOrderNbr[/code] would be called once for every valid row, trying to validate the order number of the row that was imported into the database -- where that order number is part of the row in the file and is a sequential row counter. [code]skipOrderNbr[/code] would then indicate, whether order number checking has to take place at all, [code]index[/code] would be the current row index to be checked, and [code]orderNbr[/code] would be the current order number as read back from the database after the code to be tested has executed.

    I hope this explanation is clear enough to make the context a little more understandable.


Log in to reply