Are there any problems with using an unbounded loop for flow control?

I have a project with some code which looks like the following:

L: for (;;) {
    if (someBool) {
        someOtherBool = doSomeWork();
        if (someOtherBool) {
            doFinalWork();
            break L;
        }
        someOtherBool2 = doSomeWork2();
        if (someOtherBool2) {
            doFinalWork2();
            break L;
        }
        someOtherBool3 = doSomeWork3();
        if (someOtherBool3) {
            doFinalWork3();
            break L;
        }
    }
    bitOfData = getTheData();
    throw new SomeException("Unable to do something for some reason.", bitOfData);
}
progressOntoOtherThings();

I'm wondering whether this is good programming practice? Could there be any maintainability problems with using an unbounded loop in this way for flow control? What would be a better alternative coding structure?

1 answer

  • answered 2017-11-12 22:22 Chris Nash

    It seems from the comments above (on the question) that the code was poorly written. I need to refactor it, because I've been tasked with adding further branches to the original code (I was not allowed to post the real code, so in the question I converted it to some pseudocode). This is what I ended up with:

    void method() {
        if (!someBool) {
            throwingMethod();
        }
        someOtherBool = doSomeWork();
        if (someOtherBool) {
            doFinalWork();
        } else {
            someOtherBool2 = doSomeWork2();
            if (someOtherBool2) {
                doFinalWork2();
            } else {
                someOtherBool3 = doSomeWork3();
                if (someOtherBool3) {
                    doFinalWork3();
                } else {
                    throwingMethod();
                }
            }
        }
        progressOntoOtherThings();
    }
    void throwingMethod() throws SomeException {
            bitOfData = getTheData();
            throw new SomeException("Unable to do something for some reason.", bitOfData);
    }
    

    So the Exception throwing has been moved into its own method (BTW, those lines are vastly more extensive in the actual code).

    I'm going to try changing the real code now. Fortunately we already have some unit tests, so I can be confident of the changes. However, I'm not entirely satisfied as I think I'm going to end up with quite a lot of if ... else nesting, considering the real code is vastly more complicated than the example above. (FYI, the above has 4 if branches, but the real code has probably 40 or 50 and I need to add 11 more.)