Reputation:
I found some C code that has this structure:
switch (n) {
do {
case 1:
// do some things
if (some condition)
goto go_on;
case 2:
// do some things
if (some condition)
goto go_on;
case 3:
// do some things
if (some condition)
goto go_on;
} while (1);
do {
case 4:
// do some things
if (some condition)
goto go_on;
case 5:
// do some things
if (some condition)
goto go_on;
} while (1);
}
go_on:
I programmed in C for years (many years ago) and would have thought this would be a syntax error. I think it has something to do with optimizing the loop, but I was wondering if someone could explain what it does. What happens when a while(1)
is reached, does it effectively go back to the switch? And in particular why are there two do..while's in there?
By the way, I don't want to start a discussion on the use of goto
or if this is bad design. I didn't write this and I assume the author did it this way because it made the loop run as fast as possible.
Upvotes: 15
Views: 6082
Reputation: 324
I've seen this sort of structure before. It's possible that the author was writing a state machine of some sort. (See the source to zlib for an example.)
Does the "do some stuff" part change n to whatever case will run next?
Upvotes: 0
Reputation: 46482
I'm not sure if my variant is any better, but I'd write
switch (n) {
LOOP1:
// do some things
if (some condition)
break;
case 2:
// do some things
if (some condition)
break;
case 3:
// do some things
if (some condition)
break;
goto LOOP1;
case 4:
LOOP4:
// do some things
if (some condition)
break;
case 5:
// do some things
if (some condition)
break;
goto LOOP4;
}
The original is pretty confusing because of the switch-do-while
. All replacement solutions use some goto
s, and actually many of them, and/or nested complicated conditions. So I instead replaced the confusing do-while
by a goto
.
You could simplify
if (some condition)
break;
goto LOOP4;
into
if (!some condition) goto LOOP4;
but I prefer to keep it uniform.
If you can extract
// do some things
if (some condition)
break;
into a function, the you can write
switch (n) {
case 1: while (true) {
if (body1()) break;
if (body2()) break;
if (body3()) break;
}
break;
case 2: while (true) {
if (body2()) break;
if (body3()) break;
if (body1()) break;
}
break;
case 3: while (true) {
if (body3()) break;
if (body1()) break;
if (body2()) break;
}
break;
case 4: while (true) {
if (body4()) break;
if (body5()) break;
}
break;
case 5: while (true) {
if (body5()) break;
if (body4()) break;
}
}
It's bit repetitive, but pretty clear and uses no fall-though.
Upvotes: 0
Reputation: 311048
All cases are simply labels. So if to remove the switch statement the code will look like
do {
case '1':
// do some things
if (some condition)
goto go_on;
case '2':
// do some things
if (some condition)
goto go_on;
case '3':
// do some things
if (some condition)
goto go_on;
} while (1);
do {
case '4':
// do some things
if (some condition)
goto go_on;
case '5':
// do some things
if (some condition)
goto go_on;
} while (1);
go_on:
So there are two ordinary infinite do-while loops that stop iterations depending on some internal conditions in the loops. You could write this code even simpler
do {
// do some things
if (some condition)
goto go_on;
// do some things
if (some condition)
goto go_on;
// do some things
if (some condition)
goto go_on;
} while (1);
do {
// do some things
if (some condition)
goto go_on;
// do some things
if (some condition)
goto go_on;
} while (1);
go_on:
So what does add the switch to the code shown above? It only provides entry points to the loops bypassing the normal entry points of the loops. That is the first iterations will start from some case label and nothing more. Also if there is no appropriate case label then the loops will be skipped.
Upvotes: 0
Reputation: 154280
Equivalent code that may better show program flow.
Akin to Duff's device to allow entering a loop at some intermediate position. @ tangrs
General frowned upon these days as 1) compilers typically do a better job at optimizing & 2) as OP has found, can easily obscure the meaning of the code. Use with caution.
In OP's code, after either while
condition, program flow does not return back to the switch
statement. The switch
and case
only affect the initial entry into the while
loops.
if (n == '1') goto case1;
if (n == '2') goto case2;
...
if (n == '5') goto case5;
goto go_on;
do {
case1:
// do some things
if (some condition) goto go_on;
case2:
// do some things
if (some condition) goto go_on;
case3:
// do some things
if (some condition) goto go_on;
} while (1);
do {
case4:
// do some things
if (some condition) goto go_on;
case5:
// do some things
if (some condition) goto go_on;
} while (1);
go_on:
[Edit]
There are 2 while
loops to accommodate the original coder's flow to jump into intermediate points into 1 of 2 loops.
Candidate re-write follows. Having access to the overall code, certainly a cleaner solution can be had.
int n2 = n; // Only evaluate n once as in the switch statement.
if (n2 >= 1) {
if (n2 <= 3) {
while (1) {
if (n2 <= 1) {
// do some things
if (some condition) { break; }
}
if (n2 <= 2) {
// do some things
if (some condition) { break; }
}
// do some things
if (some condition) { break; }
n2 = 1;
}
else if (n2 <= 5) {
while (1) {
if (n2 <= 4) {
// do some things
if (some condition) { break; }
}
// do some things
if (some condition) { break; }
n2 = 4;
}
}
}
Upvotes: 5
Reputation: 133059
Let me rewrite this code for you, maybe this will make it more obvious. The following code is more or less equivalent to the one you posted:
if (n == 1) goto ONE;
if (n == 2) goto TWO;
if (n == 3) goto THREE;
if (n == 4) goto FOUR;
if (n == 5) goto FIVE;
goto SKIP_ALL;
while (true) {
ONE:
// do some things
if (some condition) goto go_on;
TWO:
// do some things
if (some condition) goto go_on;
THREE:
// do some things
if (some condition) goto go_on;
}
while (true) {
FOUR:
// do some things
if (some condition) goto go_on;
FIVE:
// do some things
if (some condition) goto go_on;
}
SKIP_ALL:
go_on:
The loops are within the switch, they don't cause the switch to take place more often. The switch basically decides to which loop the program flow jumps and at which instruction within that loop it starts. Once it jumped there, the loops continue normally. Also note that a switch is usually faster than all those if
statements.
And no, goto is not bad design in general. A switch
is merely just a goto and a switch is not bad design. In reality, pretty much every code branch within a function/method executed on a CPU or within a VM is a simple goto (sometimes a conditional one, sometimes not). It's just that goto is most primitive, low level way of branching and tells the reader little about the intention. Whenever there is a higher level one, one that makes your intention more obvious, it is preferable to use that one instead. Using a goto is only bad design if you could have easily written the same kind of code without using a goto and it would not have been much worse either. In some (albeit very rare) cases a goto is almost unavoidable or any attempt to avoid it creates ugly, unreadable, very complex code or very poor performance as a result.
The "goto considered harmful" essay comes from a time, where some people were using goto for everything: For if/else code branches, for loops, for switches, to break out of loops/switches, to avoid recursion, etc. And if you overuse goto like that and if you do things like jumping to a label that immediately jumps to another label, people lose overview. Code like this is unreadable and extremely hard to debug. This is the way how you write assembly code, but that shouldn't be the way how we write C code.
Upvotes: 5