Java: Exceptions as control flow?

Question

I've heard that using exceptions for control flow is bad practice. What do you think of this?

public static findStringMatch(g0, g1) {

    int g0Left = -1;
    int g0Right = -1;
    int g1Left = -1;
    int g1Right = -1;

//if a match is found, set the above ints to the proper indices
//...
//if not, the ints remain -1

     try {
      String gL0 = g0.substring(0, g0Left);
      String gL1 = g1.substring(0, g1Left);

      String g0match = g0.substring(g0Left, g0Right);
      String g1match = g1.substring(g1Left, g1Right);

      String gR0 = g0.substring(g0Right);
      String gR1 = g1.substring(g1Right);

      return new StringMatch(gL0, gR0, g0match, g1match, gL1, gR1);
     }
     catch (StringIndexOutOfBoundsException e) {
      return new StringMatch(); //no match found
     }

So, if no match has been found, the ints will be -1. This will cause an exception when I try to take the substring g0.substring(0, -1). Then the function just returns an object indicating that no match is found.

Is this bad practice? I could just check each index manually to see if they're all -1, but that feels like more work.

UPDATE

I have removed the try-catch block and replaced it with this:

 if (g0Left == -1 || g0Right == -1 || g1Left == -1 || g1Right == -1) {
  return new StringMatch();
 }

Which is better: checking if each variable is -1, or using a boolean foundMatch to keep track and just check that at the end?

Answer 1

Generally exceptions are expensive operations and as the name would suggest, exceptional conditions. So using them in the context of controlling the flow of your application is indeed considered bad practice.

Specifically in the example you provided, you would need to do some basic validation of the inputs you are providing to the StringMatch constructor. If it were a method that returns an error code in case some basic parameter validation fails you could avoid checking beforehand, but this is not the case.

Answer 2

Yes, this is a bad practice, especially when you have a means to avoid an exception (check the string length before trying to index into it). Try and catch blocks are designed to partition "normal" logic from "exceptional" and error logic. In your example, you have spread "normal" logic into the exceptional/error block (not finding a match is not exceptional). You are also misusing substring so you can leverage the error it produces as control flow.

Answer 3

I've done some testing on this. On modern JVMs, it actually doesn't impact runtime performance much (if at all). If you run with debugging turned on, then it does slow things down considerably.

See the following for details

(I should also mention that I still think this is a bad practice, even if it doesn't impact performance. More than anything, it reflects a possibly poor algorithm design that is going to be difficult to test)

Answer 4

Program flow should be in as straight a line as possible(since even then applications get pretty complex), and utilize standard control flow structures. The next developer to touch the code may not be you and (rightly)misunderstand the non-standard way you are using exceptions instead of conditionals to determine control flow.

I am fighting a slightly different slant on this problem right now during some legacy code refactoring.

The largest issue that I find with this approach is that using the try/catch breaks normal programmatic flow.

In the application I am working on(and this is different from the sample you have applied), exceptions are used to communicate from within a method call that a given outcome(for instance looking for an account number and not finding it) occurred. This creates spaghetti code on the client side, since the calling method (during a non-exceptional event, or a normal use-case event) breaks out of whatever code it was executing before the call and into the catch block. This is repeated in some very long methods many times over, making the code very easy to mis-read.

For my situation, a method should return a value per it's signature for all but truly exceptional events. The exception handling mechanism is intended to take another path when the exception occurs (try and recover from within the method so you can still return normally).

To my mind you could do this if you scope your try/catch blocks very tightly; but I think it is a bad habit and can lead to code that is very easy to misinterpret, since the calling code will interpret any thrown exception as a 'GOTO' type message, altering program flow. I fear that although this case does not fall into this trap, doing this often could result in a coding habit leading to the nightmare that I am living right now.

And that nightmare is not pleasant.