Skip to main content
added 562 characters in body
Source Link
janos
  • 111.7k
  • 15
  • 152
  • 391

There might be good use cases for goto, but this is not one of them.

It's best when code reads from top to bottom. The goto statements break that readable flow. Since the jump-to labels are quite far, I'm forced to scroll to the end to see what they do. After I scrolled, I find some code using variables defined at the beginning, which I haven't memorized earlier, so now I have to go back to top. Effectively I'm making several jumps back and forth to understand how this works, and then a few more jumps to verify carefully that it should work well.

This is a nightmare, and it doesn't have to be. If you replace the goto statements with the code at their jump-to labels, the code becomes:

if (pipe(in)) {
    return -1;
}
if (pipe(out)) {
    close(in[0]);
    close(in[1]);
    return -1;
}
if (pipe(err)) {
    close(out[0]);
    close(out[1]);
    close(in[0]);
    close(in[1]);
    return -1;
}

This is a lot more readable: everything I need to understand these guard statements are right there above.

You probably don't like the duplication when closing in, and again later in the code if fork failed, closing in and out. It's true that code duplication is not good, but I think it's the lesser of evils here, and it's outweighed by the improvement in readability.

If you still disagree, and consider avoiding the duplication more important, then you can improve this technique with goto by using more descriptive labels, for example:

  close_err_and_out_and_in_and_return_with_failure:
    close(err[0]);
    close(err[1]);
  close_out_and_in_and_return_with_failure:
    close(out[0]);
    close(out[1]);
  close_in_and_return_with_failure:
    close(in[0]);
    close(in[1]);
  return_with_failure:
    return -1;

Avoid comments when possible. Instead of a comment, try to code in a way to make the comment unnecessary. For example:

/* This section is never be reached without a goto */

This comment could be replaced with this statement:

return pid;

Not only this is short and sweet, it has the great benefit that since it's code, it is enforced: the lines below it will not be reached, no matter what changes in the code above the return statement.

And we're kind of back to the pesky goto statements. Without the goto statements, this concern here about the comment and the logic wouldn't exist in the first place.

There might be good use cases for goto, but this is not one of them.

It's best when code reads from top to bottom. The goto statements break that readable flow. Since the jump-to labels are quite far, I'm forced to scroll to the end to see what they do. After I scrolled, I find some code using variables defined at the beginning, which I haven't memorized earlier, so now I have to go back to top. Effectively I'm making several jumps back and forth to understand how this works, and then a few more jumps to verify carefully that it should work well.

This is a nightmare, and it doesn't have to be. If you replace the goto statements with the code at their jump-to labels, the code becomes:

if (pipe(in)) {
    return -1;
}
if (pipe(out)) {
    close(in[0]);
    close(in[1]);
    return -1;
}
if (pipe(err)) {
    close(out[0]);
    close(out[1]);
    close(in[0]);
    close(in[1]);
    return -1;
}

This is a lot more readable: everything I need to understand these guard statements are right there above.

You probably don't like the duplication when closing in, and again later in the code if fork failed, closing in and out. It's true that code duplication is not good, but I think it's the lesser of evils here, and it's outweighed by the improvement in readability.


Avoid comments when possible. Instead of a comment, try to code in a way to make the comment unnecessary. For example:

/* This section is never be reached without a goto */

This comment could be replaced with this statement:

return pid;

Not only this is short and sweet, it has the great benefit that since it's code, it is enforced: the lines below it will not be reached, no matter what changes in the code above the return statement.

And we're kind of back to the pesky goto statements. Without the goto statements, this concern here about the comment and the logic wouldn't exist in the first place.

There might be good use cases for goto, but this is not one of them.

It's best when code reads from top to bottom. The goto statements break that readable flow. Since the jump-to labels are quite far, I'm forced to scroll to the end to see what they do. After I scrolled, I find some code using variables defined at the beginning, which I haven't memorized earlier, so now I have to go back to top. Effectively I'm making several jumps back and forth to understand how this works, and then a few more jumps to verify carefully that it should work well.

This is a nightmare, and it doesn't have to be. If you replace the goto statements with the code at their jump-to labels, the code becomes:

if (pipe(in)) {
    return -1;
}
if (pipe(out)) {
    close(in[0]);
    close(in[1]);
    return -1;
}
if (pipe(err)) {
    close(out[0]);
    close(out[1]);
    close(in[0]);
    close(in[1]);
    return -1;
}

This is a lot more readable: everything I need to understand these guard statements are right there above.

You probably don't like the duplication when closing in, and again later in the code if fork failed, closing in and out. It's true that code duplication is not good, but I think it's the lesser of evils here, and it's outweighed by the improvement in readability.

If you still disagree, and consider avoiding the duplication more important, then you can improve this technique with goto by using more descriptive labels, for example:

  close_err_and_out_and_in_and_return_with_failure:
    close(err[0]);
    close(err[1]);
  close_out_and_in_and_return_with_failure:
    close(out[0]);
    close(out[1]);
  close_in_and_return_with_failure:
    close(in[0]);
    close(in[1]);
  return_with_failure:
    return -1;

Avoid comments when possible. Instead of a comment, try to code in a way to make the comment unnecessary. For example:

/* This section is never be reached without a goto */

This comment could be replaced with this statement:

return pid;

Not only this is short and sweet, it has the great benefit that since it's code, it is enforced: the lines below it will not be reached, no matter what changes in the code above the return statement.

And we're kind of back to the pesky goto statements. Without the goto statements, this concern here about the comment and the logic wouldn't exist in the first place.

added 562 characters in body
Source Link
janos
  • 111.7k
  • 15
  • 152
  • 391

The most obvious thing to pick onThere might be good use cases for goto, but this is thenot one of them.

It's best when code reads from top to bottom. The goto statements break that readable flow. ThisSince the jump-to labels are quite far, I'm forced to scroll to the end to see what they do. After I scrolled, I find some code using variables defined at the beginning, which I haven't memorized earlier, so now I have to go back to top. Effectively I'm making several jumps back and forth to understand how this works, and then a few more jumps to verify carefully that it should work well.

This is not recommendeda nightmare, and it doesn't have to be. If you replace the goto statements with the code at their jump-to labels, the code becomes:

  • Readability suffers a lot when you make the reader jump around in the code: jump forward to see how the conditions was handled, then jump back again to see how the variables used there were defined. It's a nightmare.

  • Poor readability in turn invites more mistakes: when related code segments are far away in the source, you might make a change in one part that breaks the other.

if (pipe(in)) {
    return -1;
}
if (pipe(out)) {
    close(in[0]);
    close(in[1]);
    return -1;
}
if (pipe(err)) {
    close(out[0]);
    close(out[1]);
    close(in[0]);
    close(in[1]);
    return -1;
}

ThereThis is a better way to do thislot more readable: move everything I need to understand these guard statements are right there above.

You probably don't like the codeduplication when closing in, and again later in the labels to the code if gotofork pointsfailed, closing in and return thereout. Simple It's true that code duplication is not good, and a lot easier to follow what's happening but I think it's the lesser of evils here, and it's outweighed by the improvement in readability.

 

Avoid comments wheneverwhen possible. Instead of a comment, try to code in a way to make the comment unnecessary. For example:

/* This section is never be reached without a goto */

This comment could be replaced with this statement:

return pid;

Not only this is short and sweet, it has the great benefit that since it's code, it will always be enforced. A comment is powerless. The code above the original may change in a way thatenforced: the comment is no longer true, executionlines below it will continue beyond that linenot be reached, manifestingno matter what changes in bugs that may be hard to noticethe code above the return statement.

And we're kind of back to the pesky goto statements. You wouldn't have to worry about writing comments like this or figuring out a better way to do it if you hadn't been usingWithout the goto statements, this concern here about the comment and the logic wouldn't exist in the first place.

The most obvious thing to pick on is the goto statements. This is not recommended:

  • Readability suffers a lot when you make the reader jump around in the code: jump forward to see how the conditions was handled, then jump back again to see how the variables used there were defined. It's a nightmare.

  • Poor readability in turn invites more mistakes: when related code segments are far away in the source, you might make a change in one part that breaks the other.

There is a better way to do this: move the code in the labels to the goto points, and return there. Simple, and a lot easier to follow what's happening.

Avoid comments whenever possible. Instead of a comment, try to code in a way to make the comment unnecessary. For example:

/* This section is never be reached without a goto */

This comment could be replaced with this statement:

return pid;

Not only this is short and sweet, it has the great benefit that since it's code, it will always be enforced. A comment is powerless. The code above the original may change in a way that the comment is no longer true, execution will continue beyond that line, manifesting in bugs that may be hard to notice.

And we're kind of back to the pesky goto statements. You wouldn't have to worry about writing comments like this or figuring out a better way to do it if you hadn't been using goto.

There might be good use cases for goto, but this is not one of them.

It's best when code reads from top to bottom. The goto statements break that readable flow. Since the jump-to labels are quite far, I'm forced to scroll to the end to see what they do. After I scrolled, I find some code using variables defined at the beginning, which I haven't memorized earlier, so now I have to go back to top. Effectively I'm making several jumps back and forth to understand how this works, and then a few more jumps to verify carefully that it should work well.

This is a nightmare, and it doesn't have to be. If you replace the goto statements with the code at their jump-to labels, the code becomes:

if (pipe(in)) {
    return -1;
}
if (pipe(out)) {
    close(in[0]);
    close(in[1]);
    return -1;
}
if (pipe(err)) {
    close(out[0]);
    close(out[1]);
    close(in[0]);
    close(in[1]);
    return -1;
}

This is a lot more readable: everything I need to understand these guard statements are right there above.

You probably don't like the duplication when closing in, and again later in the code if fork failed, closing in and out. It's true that code duplication is not good, but I think it's the lesser of evils here, and it's outweighed by the improvement in readability.

 

Avoid comments when possible. Instead of a comment, try to code in a way to make the comment unnecessary. For example:

/* This section is never be reached without a goto */

This comment could be replaced with this statement:

return pid;

Not only this is short and sweet, it has the great benefit that since it's code, it is enforced: the lines below it will not be reached, no matter what changes in the code above the return statement.

And we're kind of back to the pesky goto statements. Without the goto statements, this concern here about the comment and the logic wouldn't exist in the first place.

Source Link
janos
  • 111.7k
  • 15
  • 152
  • 391

The most obvious thing to pick on is the goto statements. This is not recommended:

  • Readability suffers a lot when you make the reader jump around in the code: jump forward to see how the conditions was handled, then jump back again to see how the variables used there were defined. It's a nightmare.

  • Poor readability in turn invites more mistakes: when related code segments are far away in the source, you might make a change in one part that breaks the other.

There is a better way to do this: move the code in the labels to the goto points, and return there. Simple, and a lot easier to follow what's happening.

Avoid comments whenever possible. Instead of a comment, try to code in a way to make the comment unnecessary. For example:

/* This section is never be reached without a goto */

This comment could be replaced with this statement:

return pid;

Not only this is short and sweet, it has the great benefit that since it's code, it will always be enforced. A comment is powerless. The code above the original may change in a way that the comment is no longer true, execution will continue beyond that line, manifesting in bugs that may be hard to notice.

And we're kind of back to the pesky goto statements. You wouldn't have to worry about writing comments like this or figuring out a better way to do it if you hadn't been using goto.