Throw when succeeding vs. do-while-false vs. function array vs. goto statements

Question

Consider a class that is supposed to make parameter suggestion, given some clues, and a specific acceptance test.

Example to concretise: Say you are guessing the cubic dimensions of a raw data file, based on the filename. The acceptance test is: total elements == file-size (assuming 1 byte pr. grid unit).

This requires a prioritized ordering of tests, where each test does one or more attempt to pass the acceptance test. The first suggestion that passes is immediately returned, and no more attempts are made. If none pass, suggest nothing.

The question: Which pattern/approach would you recommend, when readability is the main concern? Also, what are the flaws and drawbacks with the following suggestions?

Method 1: Exceptions for catching a successful acceptance test

I've heard said by wise people to avoid using try/catch when not catching actual exceptions. However, in this case, the result is fairly readable, and looks something like:

try {
  someTest1();
  someTest2();
  // ...
  someTestN();
}
catch(int){
  // Succesfull return
  xOut = x_; yOut = y_; zOut = z_;
  return;
}
xOut = -1; yOut = -1; zOut = -1;

With the inner acceptance test:

void acceptanceTest(const int x, const int y, const int z)
{
  if (verify(x * y * z)) {
    x_ = x;   y_ = y;  z_ = z;
    throw 1;
  }
}

Method 2: Do-while-false:

Change: All tests return true as soon it passes the acceptance test. Returns false if all tries in the test fails.

do {
  if ( someTest1() ) break;
  if ( someTest2() ) break;
  // ...
  if ( someTestN() ) break;
  // All tests failed
  xOut = -1; yOut = -1; zOut = -1;
  return;
} while (0);
xOut = x_; yOut = y_; zOut = z_;

Acceptance test:

bool acceptanceTest(const int x, const int y, const int z)
{
  if (verify(x * y * z)) {
    x_ = x;   y_ = y;  z_ = z;
    return true;
  }
  return false;
}

Method 3: Array of function pointers

typedef bool (TheClassName::*Function)();
Function funcs[] = { &TheClassName::someTest1,
                     &TheClassName::someTest2,
                     // ...
                     &TheClassName::someTestN };

for (unsigned int i = 0; i < sizeof(funcs)/sizeof(funcs[0]); ++i) {
  if ( (this->*funcs[i])() ) {
    xOut = x_;  yOut = y_;  zOut = z_;
    return;
  }
}
xOut = -1;  yOut = -1;  zOut = -1;

Test functions and acceptance test the same as for do-while-false.

Method 4: Goto

I've seen the do-while-false referred to as a disguised goto, followed by the argument that if that's the intended behavior "why not use goto?". So I'll list it up:

if (someTest1() ) goto success;
if (someTest2() ) goto success;
// ...
if (someTestN() ) goto success;
xOut = -1;  yOut = -1;  zOut = -1;
return;

success:
xOut = x_;  yOut = y_;  zOut = z_;
return;

Test functions and acceptance test the same as for do-while-false.

Method 5: Short-circuiting logic (suggested by Mike Seymour)

if (someTest1() ||
    someTest2() ||
    // ...
    someTestN()) {
    // success    
  xOut = x_;  yOut = y_;  zOut = z_;
  return;
}
xOut = -1;  yOut = -1;  zOut = -1;

Test functions and acceptance test the same as for do-while-false.

Edit: I should point out that Methods 2,3,4,5 differ from 1 by requiring boolean return value on the acceptance test that is passed all the way back to the return function, as well as added overhead in each test function that does multiple attempts at passing the acceptance test.

This makes me think that method 1 has an advantage to maintainability as the control logic is solely at the bottom level: the acceptance test.

Answer to your edit about Method 1: you're trying to break the normal flow of the execution, and that's against structured programming. You can reduce overhead with GOTOs but that's (as everyone knows) evil. Using exceptions like this will go to a nightmare sooner or latter (not to mention what other will think when they have to maintain your code and do not understand why there is an exception if nothing is wrong). — Ignacio Soler Garcia, Nov 09 '11 at 15:12
I agree 100% that those are the main issues. However, there are arguably some benefits. If, in the future, the acceptance test needed to be three states (say [pass A| pass B| fail] instead of the two: [pass | fail]), the control flow becomes non-trivial for all the listed methods, _except_ Method 1. Not only that, but the only change needed is in the acceptance test itself. This surely is a plus on the maintainability of method 1, right? — swalog, Nov 09 '11 at 15:19
You know, the is absolutely nothing wrong with the GOTO statement and it can increase the maintainability of some code ... but I can say it a good thing to do. And when you say its a plus on the maintainability ... its only for you, others won't expect that and then will be a minus. — Ignacio Soler Garcia, Nov 09 '11 at 17:07

Mike Seymour · Accepted Answer · 2011-11-09T12:18:04.337

Method 5: Short-circuiting logic

if (someTest1() ||
    someTest2() ||
    // ...
    someTestN()) 
{
    // success    
}

This is equivalent to (and in my opinion easier to follow than) options 2 and 4, which emulate the short-circuiting behaviour with other flow control operations.

Option 3 is very similar, but a bit more flexible; it might be a good idea if you need to apply the same pattern to different sets of tests, but is overkill if you just have a single set of tests.

Option 1 will be rather surprising to a lot of people, since exceptions are generally only used for unexpected events; although, if the tests are structured so that detecting success happens somewhere down a deep call chain, then this might be more convenient than passing a return value back up. It will certainly need documenting, and you should throw a type with a meaningful name (e.g. success), and be careful that it won't be caught by any error-handling mechanism. Exceptions are usually much slower than normal function returns, so bear that in mind if performance is an issue. Having said all that, if I were tempted to use exceptions here, I would certainly be looking for ways to simplify the structure of the tests to make a return value more convenient.

Interesting. Cause it's evaluated left-to-right, and the first to return true suffices and it stops. However, in terms of readability, this would rest upon the readers knowledge of order-of evaluation. Still, I like it. Thanks. — swalog, Nov 09 '11 at 11:33
@EXIT_FAILURE : I think this kind of short circuiting is basic knowledge. If I write code I assume every programmer knows this. — rve, Nov 09 '11 at 11:59
@rve: I agree on both accounts, and have added it to the post. — swalog, Nov 09 '11 at 12:06

score 3 · Answer 2 · answered Nov 09 '11 at 11:30

Well, I really would go for the simplest and easy solution:

bool succeeded;

if (!succeeded)
    succeeded = someTest1();
if (!succeeded)
    succeeded = someTest2();
if (!succeeded)
    succeeded = someTest3();
if (!succeeded)
    succeeded = someTestN();

I can argue the other solutions but sumarizing: simple good, complicated bad.

score 0 · Answer 3 · answered Nov 09 '11 at 11:35

0

I think that first method is more readable, c++ based and easiest to maintain than the others. Gotos and do-while-false adds a little bit of confusion. There are variants about all of these methods, but I prefer the first one.

answered Nov 09 '11 at 11:35

Tio Pepe

3,021
1
14
21

That was my initial thought as well, and how I implemented it. This was met with somewhat disapproval of the more experienced programmers (based on sound arguments, that try/catch should only catch _exceptions_, and other low level stack-details that occur when throwing), and just for fun, I'd try to solve it in as many ways possible, and evaluate the readability and maintainability. – swalog Nov 09 '11 at 11:40
But, besides, you can mix and improve this method using boolean return values as others methods. As you defined acceptance testes, out of tested functions, there is no impact on them. About low level stack-details, this is true. You are modifying the tested environment but, in your own tests are doing more or less the same, adding libraries, unit testing frameworks, etc. – Tio Pepe Nov 09 '11 at 11:52
I think exceptions are bad for normal flow, see also http://stackoverflow.com/q/729379/79455 – rve Nov 09 '11 at 12:03

Throw when succeeding vs. do-while-false vs. function array vs. goto statements

3 Answers3