Is it bad form to use lots of break(s) in C++?

[mv2devnull]

Dirty little variant from iCyborg's:

// destructor does no close MyFile, so resort to macro
#define BREAKOUT { fp.close(); return ret; }

bool function(int ID)
{
    MyFile fp;
    bool ret = false;

    fp.open("filename");

    int index = fp.findIndex(ID);
    if (index == IndexNotFound) BREAKOUT
        
    int x = fp.readX(index);
    if (x<0 || x>100) BREAKOUT

    int y = fp.readY(index);
    if (y == 0) BREAKOUT

    fp.writeDiv(x/y);
    ret = true;
    BREAKOUT
}

 

[douglasb]

No, this is not good, if you could do that, you wouldn't need a new function at all, just a compound if.
The problem is you can't separate code from conditions, there's code0 in the beginning, then codeA that should only be executed if condA is true, then codeB etc.
Say if you have something like

doStuff1();
a = Factory::CreateA();
if (a == NULL) break;
a->init()

You can't just move everything to:

doStuff1();
a = Factory::CreateA();
a->init()

and then check at the end if a is not null, you'll segfault.

So wouldn't you simply have codeA, codeB, codeC, etc. simply return true, and add them to the if statement like so:

if (condA && codeA && condB && codeB && condC && codeC) {
 whatever();
}

Been a while since I've done anything in C++, but it does have the short-circuit AND, correct?

 

[iCyborg]

Yes you can. But try with the example from two posts above and it will be very ugly. Code A needs to receive a bunch of parameters, e.g. to read x, you also need to pass in an address to which it will be stored (if it is to return boolean), not to mention that you will be breaking up a homogenous function into a number of small pieces adding quite significantly to code complexity and simply reducing readability.

 

[douglasb]

Good point. But then again, I don't know that anybody ever chose C++ for its readability and lack of complexity.

 

[Pia]

C++ can be very readable. The spaghetti you guys are cooking is C.

bool function() {
  auto g1 = scope_guard([&]{ ... cleanup here ... });
  if (cond1) return;
  ...
  if (cond2) return;
  ...
}
bool same_function_using_handy_macro() {
  AT_SCOPE_EXIT { ... cleanup here ... }
  if (cond1) return;
  ...
  if (cond2) return;
  ...
}

Like iCyborg noted, for straightforward resource allocations/releases good C++ code would do no explicit cleanup at all, because the resources should clean up after themselves. Scope guards are for handling the special cases where the cleanup needs more than basic destruction.

This is not only easier to read than the C way, and harder to make mistakes with, but also inherently exception-safe.

 

[iCyborg]

Like iCyborg noted, for straightforward resource allocations/releases good C++ code would do no explicit cleanup at all, because the resources should clean up after themselves. Scope guards are for handling the special cases where the cleanup needs more than basic destruction.

This is not only easier to read than the C way, and harder to make mistakes with, but also inherently exception-safe.

I was trying to put up a minimal example, and in that case you're right about the bolded part. But generally this will not be always possible: if something is allocated on the heap, you need to explicitly deallocate it, destructor won't be called when you go out of function scope. Sometimes you can make RAII style wrappers, but often it's very impractical. Or perhaps there's a function call to send notification or something that must always be executed...

 

[Pia]

Sometimes you can make RAII style wrappers, but often it's very impractical. Or perhaps there's a function call to send notification or something that must always be executed...

Yes, those are exactly the kind of situations where you should use a scope guard.

void function() {
AT_SCOPE_EXIT { always_send_this_notification(); }
if (return_early())
    return;
this_call_could_throw_an_exception();
return;
} // doesn't matter how we exited the scope, the notification got sent

 

[dighn]

auto g1 = scope_guard([&]{ ... cleanup here ... });

is that c++11? while that looks very elegant, I'm not sure if it's as good without lambda closures.

but of course if you are doing C++, you should be relying on RAII, in theory anyway. in practice if you are dealing with external libraries that don't support it, you have to write your own wrappers for everything or use the C way.

 

[Pia]

auto g1 = scope_guard([&]{ ... cleanup here ... });

is that c++11? while that looks very elegant, I'm not sure if it's as good without lambda closures.

The auto and lambda are C++11, and indeed they make scope guards more convenient to write, but the technique is not new. Here's an article from 13 years ago presenting scope guards in C++98: http://www.drdobbs.com/cpp/generic-change-the-way-you-write-excepti/184403758

For that matter, all major C++ compilers support those features and have done so for a long time. Even VS2010, while having no C++11 support as such, does support auto and lambdas and has done so way before the standard. The people who can and should use C++11 features are now the majority.

but of course if you are doing C++, you should be relying on RAII, in theory anyway. in practice if you are dealing with external libraries that don't support it, you have to write your own wrappers for everything or use the C way.

Or you could use the standard C++ technique I've been showing for the last two posts.

... code ...

// Using C-style library just once. No C spaghetti. No wrapper.
FILE* file = fopen("data");
AT_SCOPE_EXIT { fclose(file); };

... code ...

 

[dighn]

granted, if you have access to c++11 features and are not likely to experience portability issues from their use, then it is the superior choice. that's not always true though (e.g. at my workplace), and without lambdas the scope guard pattern seems cumbersome.

 

[Cogman]

Breaks and return statements aren't all that bad. Gotos are bad mostly because whatever you are doing with a goto you could change into a function that may be useful in the future.

The place were this stuff becomes a mess is when you have very large functions doing very complex things. In those cases, multiple returns and breakpoints all over the place become a nightmare.

 

[Pia]

granted, if you have access to c++11 features and are not likely to experience portability issues from their use, then it is the superior choice. that's not always true though (e.g. at my workplace), and without lambdas the scope guard pattern seems cumbersome.

I'm curious - what's keeping you from adopting the new standard at least in part?

Even in C++98, scope guards are handy. The previous example is no harder to write:

FILE* file = fopen("data");
ON_SCOPE_EXIT(fclose, file);

The difference is we are limited to calling single functions whereas C++11 lets us write arbitrary code.

 

[dighn]

we still use VS2008 for Windows. however it's not just a matter of upgrading because the majority of the code we write needs to compile on multiple platforms, including an embedded environment (based on some TI processor), whose compliance to even c++98 is suspect; we actually minimize the use of C++ (favoring C), except for GUI apps. I do admit this may not be the most common of scenarios but it is what it is. neat trick though and will try to use in code where I can.

 

[iCyborg]

Yes, those are exactly the kind of situations where you should use a scope guard.

void function() {
AT_SCOPE_EXIT { always_send_this_notification(); }
if (return_early())
    return;
this_call_could_throw_an_exception();
return;
} // doesn't matter how we exited the scope, the notification got sent

What if say file opening failed, and I do really want to return immediately with no cleanup whatsoever?

I don't really like the macro solution. You'd need one for every function requiring such behavior, and using macros is generally discouraged, and for a good reason, as they come with a lot of gotchas.

I spent some time looking at this scope guard, an interesting feature, but I generally feel that many of C++11 additions are adding complexity to an already complex language. If clarity and maintainability/readability is important, it may be a while before these become familiar to most people. It's fine if you're working on your own or in a small team, but in a large team you may want to go for simplicity.
Though I really like lambda expressions in C#. Probably my theoretical background asserting itself...

And I'm probably in the minority who can't use this. We can't even use STL and exceptions...

 

[Pia]

What if say file opening failed, and I do really want to return immediately with no cleanup whatsoever?

The most obvious thing is to construct the scope guard after the situation where you didn't want the cleanup to trigger. You can probably do this most of the time.

If the operations can't be ordered like that, or it isn't convenient to do so, you can manually disable any specific guards you want by calling .dismiss() on them right before your special case return.

You can also use this tech to do transactions. For every operation you do successfully, you just set up a corresponding guard which rolls back that operation. And at the end, when the entire transaction has successfully completed, you dismiss all the guards and allow the result to stand.

I don't really like the macro solution. You'd need one for every function requiring such behavior

I'm not sure what you mean. That macro can be used as many times as you want in any number of locations, and filled in with arbitrary cleanup code (C++11) or single arbitrary function and arguments (C++98) each time.

Anyway, if you are totally allergic to macros, the non-macro version (shown in my first post) is only slightly less convenient.

If clarity and maintainability/readability is important, it may be a while before these become familiar to most people.

Does not compute. Clarity, maintainability and readability are all reasons to favor scope guards over spaghetti / accordion cleanup.

 

[iCyborg]

Anyway, if you are totally allergic to macros, the non-macro version (shown in my first post) is only slightly less convenient.
Does not compute. Clarity, maintainability and readability are all reasons to favor scope guards over spaghetti / accordion cleanup.

I was referring to the m2devnull's post (#26). I don't really understand what your AT_SCOPE_EXIT does, is it defined somewhere else and how, or is it something standard? And I consider myself an experienced C++ programmer.

I disagree about spahetti. It's not elegant, but it's very easy to understand: while loop is artificial so we can break out, clean up and return. It's also much shorter and less convoluted. A 1st year college student should have no problems understanding it. But if you show him/her your code, he'll probably wonder if this even compiles. See my previous paragraph where I didn't understand what was going on until spending like a good 1-2 hours on C++ spec...

 

[Merad]

I was referring to the m2devnull's post (#26). I don't really understand what your AT_SCOPE_EXIT does, is it defined somewhere else and how, or is it something standard? And I consider myself an experienced C++ programmer.

I disagree about spahetti. It's not elegant, but it's very easy to understand: while loop is artificial so we can break out, clean up and return. It's also much shorter and less convoluted. A 1st year college student should have no problems understanding it. But if you show him/her your code, he'll probably wonder if this even compiles. See my previous paragraph where I didn't understand what was going on until spending like a good 1-2 hours on C++ spec...

Your last paragraph really illustrates the problem with a good many C++ programmers. If you're using well written standard-compliant good then you generally don't need to know precisely how it works, just how to use it.

To steal your example, your average college freshman (who probably isn't learning C++, BTW) hasn't the faintest idea how cout or System.out.print() works with console output buffers or interfaces with the OS, or any of that jazz. Nor do they have the slightest idea how new works to locate a block of free memory, "allocate" it, and construct an object on it. Yet somehow they're perfectly capable of using both constructs. They'd be perfectly capable of using scope guards *if* they were taught to do so.

BTW, if you want to know how scope guards work start with the Dr Dobbs article already linked, then go to this updated version: http://www.zete.org/people/jlehrer/scopeguard.html

 

[Pia]

I was referring to the m2devnull's post (#26).

Ah. I thought you were talking about my macros since you were quoting me.

I don't really understand what your AT_SCOPE_EXIT does, is it defined somewhere else and how, or is it something standard? And I consider myself an experienced C++ programmer.

The macro isn't built-in, just a small helper for building the the scope guard. My first post shows what the C++11 version looks like if you write manually without a macro ("auto g1 = ... "); the macro just expands to the equivalent of that.

The code in the C++98 version is significantly more verbose, so using a macro for it makes a much larger difference in usability than it does for the C++11 version.

Here's modern C++11 scope guard (slide 31 onwards), with a comparison to the older error handling styles that IMO makes a rock solid case for using the modern style.
Also has implementation detail of the (optional) helper macro.
http://sdrv.ms/RXjNPR

I disagree about spahetti. It's not elegant, but it's very easy to understand: while loop is artificial so we can break out, clean up and return. It's also much shorter and less convoluted.

I don't mind the do{...}while(false) hack if the alternative is a rat's nest of if/else, but would far prefer to avoid both. Neither of them scales well.

 

[iCyborg]

Your last paragraph really illustrates the problem with a good many C++ programmers. If you're using well written standard-compliant good then you generally don't need to know precisely how it works, just how to use it.

To steal your example, your average college freshman (who probably isn't learning C++, BTW) hasn't the faintest idea how cout or System.out.print() works with console output buffers or interfaces with the OS, or any of that jazz. Nor do they have the slightest idea how new works to locate a block of free memory, "allocate" it, and construct an object on it. Yet somehow they're perfectly capable of using both constructs. They'd be perfectly capable of using scope guards *if* they were taught to do so.

Some of the students do learn c++: we get 2-3 student interns almost every semester, and we only use C++.
And you shouldn't generalize quickly: I know my situation is probably an outlier, but the software product I work on involves hundreds of people. Not everyone should edit all code of course, but even my team is sized at about 40-50. If I update the module I work on after a month, the number of files changed will be in hundreds. Given that, I really prefer if everybody knows how it works, and not just how to use it

 

[iCyborg]

The macro isn't built-in, just a small helper for building the the scope guard. My first post shows what the C++11 version looks like if you write manually without a macro ("auto g1 = ... "); the macro just expands to the equivalent of that.

Well, your example isn't complete. AT_SCOPE_EXIT isn't a C++ keyword, so it better be defined earlier, nor do I understand what relation does it have with returns. I'm missing some pieces, and I don't quite know how to fill those with what I have in a way that makes it different than mv2devnull.

 

[mv2devnull]

Well, your example isn't complete. AT_SCOPE_EXIT isn't a C++ keyword, so it better be defined earlier, nor do I understand what relation does it have with returns. I'm missing some pieces, and I don't quite know how to fill those with what I have in a way that makes it different than mv2devnull.

My dirty version was just the trivial use of macro to place recurring code to various places with preprocessor. Just the type that has every right to be discouraged.

I did not know AT_SCOPE_EXIT, but it is clearly different. After reading Alexandrescu's slides it is easier to explain why.

The dirty (pseudo):

void cleanup() {...}
#define EVIL cleanup()

void foo() {
  <action1>
  EVIL;
  return;

  <action2>
  EVIL;
  return;
}

expands to boring:

void cleanup() {...}

void foo() {
  <action1>
  cleanup();
  return;

  <action2>
  cleanup();
  return;
}

The AT_SCOPE_EXIT:

#include <scope_guarding_implementation>
void cleanup() {...}

void foo() {
  AT_SCOPE_EXIT( cleanup );

  <action1>
  return;

  <action2>
  return;
}

expands to:

template <class Fun>
class Guard {
  Fun f_;
public:
  Guard( Fun f ) : f_( f ) {}
  ~Guard() { f_(); }
};

void cleanup() {...}

void foo() {
  Guard g( cleanup );

  <action1>
  return;

  <action2>
  return;
}

On every possible exit from scope the destructor of 'g' will do its duty.