1

This question already has an answer here:

Currently I am using gotos for closing handles and general cleanup if an error occurs.
Since I don't want to nest all the ifs and a function for cleanup would require a return besides the cleanup(); call, I don't have any good ideas left.

The structure is like so:

if (condition1) {
   goto cleanup;
}

if (condition2) {
   goto cleanup;
}

if (condition3) {
   goto cleanup;
}

// do stuff only if all checks passed

cleanup:
CloseHandle(x);
CloseHandle(y);
// etc.

marked as duplicate by Bart van Ingen Schenau, GlenH7, durron597, World Engineer Mar 24 '15 at 21:55

This question has been asked before and already has an answer. If those answers do not fully address your question, please ask a new question.

  • ...see also: Is this a decent use-case for goto in C? – gnat Mar 24 '15 at 12:08
  • 12
    Just use goto. It makes the intent of the code clear and is cleaner than camouflaging the control flow by any misguided attempt to cheat around using the goto statement. I really dislike the idea of blindly following “never do X” rules when X actually is the best (least bad) alternative in a given case. This particular pattern (goto cleanup;) is actually quite idiomatic in C. – 5gon12eder Mar 24 '15 at 12:21
  • 2
    There was a study (pre-print, not peer-reviewed) on the use of goto in C that concluded that this is the case where they're being used and that applications which cause spaghetti are relatively rare. – Blrfl Mar 24 '15 at 16:20
  • 1
    This is basically using goto to implement a custom control structure. Go recognized this for what it is - a control structure - and added the defer statement to cover this (valid) use-case. – Riking Mar 24 '15 at 17:59
6

The cleanup could be in an outer function, and then return can be used instead of goto:

void main_func() {
    /* Set-up goes here */
    handle x = ...;
    handle y = ...;
    void result = inner_func(x, y);

    /* Clean-up goes here */
    CloseHandle(x);
    CloseHandle(y);
}

void inner_func(x, y) {
    if (condition1) return;
    if (condition2) return;
    if (condition3) return;

    /* Do things here */
}

But goto for things like this isn't that bad, in my opinion.

  • ok, that could actually be a possibility. – Benjoyo Mar 24 '15 at 12:24
  • The slight problem I have with this is the same that I have with manlio's third solution. – Benjoyo Mar 24 '15 at 12:36
  • 1
    @KinjalDixit: the function wouldn't be meant to be called from elsewhere. My C is too rusty to suggest how to handle that -- maybe start the name with an underscore, don't put it in the .h file, et cetera. – RemcoGerlich Mar 24 '15 at 13:26
  • 1
    @Konamiman but it isn't as good as the goto solution, since you shouldn't call the other functions when one before failed. – Benjoyo Mar 24 '15 at 15:51
  • 1
    @RemcoGerlich: You're looking for static to make the function invisible to the outside. A good compiler would take care of inlining it, which would result in the same code as a goto without the overhead of a function call. – Blrfl Mar 24 '15 at 16:23
4

You could try saying what you mean:

if (!condition1
 && !condition2
 && !condition3 ) {

// do stuff only if all checks passed
}
CloseHandle(x);
CloseHandle(y);
// etc.

As various commenters have pointed out this is only readable/maintaibable if the condition tests are fairly simple where complex conditions are involved something like this is cleaner:-

if (doAble()) {
   // usefull work here
}
CloseHandle(x);
CloseHandle(y);

boolean doAble() {
  if (condition1) {
     return false;
  }
  if (condition2) {
     return false;
  }
  if (condition3) {
     return false;
  }
  return true;
}

However if you need lots of local variables to evaluate the conditions this too can get messy.

  • Wow, saying what you mean is also simpler and clearer than the alternatives. Who would have thunk it? – user949300 Mar 24 '15 at 17:27
  • Maybe this is really a good option, since evaluation will stop if one condition is false, right? – Benjoyo Mar 24 '15 at 17:39
  • Which also gets you a hard-to-reason about chain of negatives, nowhere to put any condition-specific logic or logging, and some free indentation. It's fine in the case shown, but IMO scales poorly as the number (or complexity) of conditions grows. – Useless Mar 24 '15 at 18:53
  • If the number or complexity of conditions grows, it should be broken into smaller pieces and separate functions. – Rob K Mar 24 '15 at 20:57
3

In C a typical way to simplify error checking and avoid deep nested if is:

do
{
  if (condition1) break;

  /* 1. do something... */

  if (condition2) break;

  /* 2. do something else... */

  if (condition3) break;

  /* 3. do something else... */
} while(0);

/* Cleanup */

There are various opinion on this "idiom" (e.g. take a look at Do you consider this technique “BAD”?).

Possibly you should add a comment on the do to make it clear to anyone what's happening.

You can often rewrite the code using a helper function and change the fake do-loop into:

void func(X *x, Y *y)
{
  if (condition1) return;

  /* 1. do something... */

  if (condition2) return;

  /* 2. do something else... */

  if (condition3) return;

  /* 3. do something else... */
}


/* ... */
X x;
Y y;

func(&x, &y);

/* Cleanup */

and it won't be considered a "bad practice" since it's more expected.

If you haven't the intermediate steps (1. and 2.) this is probably enough:

int stop = condition1 || condition2 || condition3;

if (!stop)
{
  /* ... */
}

/* Cleanup */
  • Good idea, but this would require a function or goto for the things to be done if all checks passed. – Benjoyo Mar 24 '15 at 12:12
  • @Benjoyo: that other stuff could go in the /* do something else */ part, right? And what's wrong with putting the stuff in a function? – RemcoGerlich Mar 24 '15 at 12:15
  • @RemcoGerlich I would need to pass a ton of handles and so on. Also this isn't really such a good SoC that I would like to use a function. – Benjoyo Mar 24 '15 at 12:27
  • Your third suggestion is an option, I didn't consider something like this because I didn't want to, if the first function fails, force the following ones to try their luck even if it's clear that they will fail. – Benjoyo Mar 24 '15 at 12:33
  • The first idea abuses the while/break language feature, goto can be considered more explicit and cleaner in such a case. – johannes Mar 24 '15 at 15:53

Not the answer you're looking for? Browse other questions tagged or ask your own question.