views:

390

answers:

10

Generally, it is good practice to avoid GOTOs. Keeping that in mind I've been having a debate with a coworker over this topic.

Consider the following code:

Line:
    while( <> ) {
        next Line if (insert logic);
    }

Does using a loop label count as a goto?

Here is what perlsyn in perldoc has to say:

Here's how a C programmer might code up a particular algorithm in Perl:

for (my $i = 0; $i < @ary1; $i++) {
    for (my $j = 0; $j < @ary2; $j++) {
        if ($ary1[$i] > $ary2[$j]) {
            last; # can't go to outer :-(
        }
        $ary1[$i] += $ary2[$j];
    }
    # this is where that last takes me
}

Whereas here's how a Perl programmer more comfortable with the idiom might do it:

OUTER: for my $wid (@ary1) {
    INNER:   for my $jet (@ary2) {
                 next OUTER if $wid > $jet;
                 $wid += $jet;
             }
       }

My take on this is no because you are explicitly telling a loop to short circuit and advance however my coworker disagrees, says that it is just a fancy GOTO and should be avoided. I'm looking for either a compelling argument or documentation that explains why this is or is not a GOTO. I'll also accept an explanation for why this is or is not considered good practice in perl.

+24  A: 

Dijkstras intent was never that anything resembling goto is to be considered harmful. It was that the structure of code where gotos are used as the main construct for almost any kind of program flow change will result in what we today call spaghetti code.

You should read the original article and keep in mind that it was written in 1968 when labeled jumps was the main flow control constructs in just about all programming languages.

http://userweb.cs.utexas.edu/users/EWD/ewd02xx/EWD215.PDF

kasperjj
haven't seen the article before thanks for the read.
stocherilac
+1, That quote has been taken out of context since he made it. It was an argument for structured programming (aka: if, else, for, while) rather than using goto for everything.
Eric Strom
What's amazing is that "structured programming" has since become such the norm, that most people don't know that there's anything else. This is a Good Thing. :)
Robert P
+2  A: 

Who cares whether it counts as goto as long as it makes the code easier to understand? Using goto can often be MORE readable than having a bunch of extra tests in if() and loop conditions.

zvrba
@Downvoters: Why was this downvoted? He just said that GOTO has its place and can, correctly used, be a good thing. Just saying GOTO is evil just because it is called GOTO is plain stupid.
codymanix
Because it doesn't add anything to the discussion. *Who cares about style as long as it makes code easier to understand.* Being the goal of style is a moot statement.
Evan Carroll
+12  A: 

The danger of GOTO labels is that they create spaghetti code and make the logic unreadable. Neither of those will happen in this case. There is a lot of validity in using GOTO statements, much of the defense coming from Donald Knuth [article].

Delving into the differences between your C and Perl example... If you consider what is happening at the assembly level with your C programs, it all compiles down to GOTOs anyway. And if you've done any MIPS or other assembly programming, then you've seen that most of those languages don't have any looping constructs, only conditional and unconditional branches.

In the end it comes down to readability and understandability. Both of which are helped an enormous amount by being consistent. If your company has a style guide, follow that, otherwise following the perl style guide sounds like a good idea to me. That way when other perl developers join your team in the future, they'll be able to hit the ground running and be comfortable with your code base.

MikeD
I haven't seen that article either, thanks looks interesting.
stocherilac
+2  A: 

I think the distinction is somewhat fuzzy, but here's what the goto perldoc states about the (frowned upon) goto statement:

The goto-LABEL form finds the statement labeled with LABEL and resumes execution there.

...

The author of Perl has never felt the need to use this form of goto (in Perl, that is; C is another matter). (The difference is that C does not offer named loops combined with loop control. Perl does, and this replaces most structured uses of goto in other languages.)

The perlsyn perldoc, however, says this:

The while statement executes the block as long as the expression is true. The until statement executes the block as long as the expression is false. The LABEL is optional, and if present, consists of an identifier followed by a colon. The LABEL identifies the loop for the loop control statements next, last, and redo. If the LABEL is omitted, the loop control statement refers to the innermost enclosing loop. This may include dynamically looking back your call-stack at run time to find the LABEL. Such desperate behavior triggers a warning if you use the use warnings pragma or the -w flag.

The desperate behaviour bit doesn't look too good to me, but I may be misinterpreting its meaning.

The Learning Perl book (5th edition, page 162) has this to say:

When you need to work with a loop block that's not the innermost one, use a label.

...

Notice that the label names the entire block; it's not marking a target point in the code. [This isn't goto after all.]

Does that help clear things up? Probably not... :-)

Mike
"looking back your call-stack" means jumping out of the current sub into some calling sub that had an appropriate loop. Such a use of next/last/redo is indeed not too good, whether a label is involved or not.
ysth
+2  A: 

Labeled loop jumps in Perl are GOTOs as much as C's break and continue are.

John
That doesn't answer the spirit of the OP's question though.
Ether
A: 

gotos are bad because they create hard to understand code--particularly, what is often called "Spaghetti Code". What's hard to understand about next Line...??

You can call it a loop "name", and it really is something to help emphasize loop boundaries. You're not jumping into an arbitrary point in relation to the loop; you're going back to the top of a loop.

Sadly enough, if it is a group or house standard, there might be nothing to convince the group that it's not a goto. I had a manager who absolutely insisted that a ternary operator made things hard to read, and preferred I use if-blocks for everything. I had a pretty good argument anything can be done in the clauses of an if-else, but that a ternary made it explicit that you were looking for a particular value. No sale.

Axeman
+1  A: 

This kind of jump is a disciplined used of a goto-like statement. So it's certainly less harmful than undisciplined use of goto. (As kasperjj wrote, "Dijkstras intent was never that anything resembling goto is to be considered harmful.")

IMO, this Perl kind of jump is even better design than C's "break" and "continue", because it makes clear what loop we break or continue, and it makes it more solid in the face of code changes. (Besides, it also allows to break or continue an outer loop.)

There are pundits who don't like break/continue and the like, but at some point there is a tradeoff to make between rules of thumb and readability, and a well-chosen break/continue or even goto may become more readable than "politically correct" code.

+1  A: 

break/last and continue/next ARE gotos. I don't understand why anyone would go to such lengths to avoid a keyword yet use a different keyword that does the same thing...

frankc
`label` isn't goto's -- they aren't even really controlled jumps. And, your *different means to accomplishing the same task* argument is like making the statement *I don't understand why people waste time to kill cancer, when they can simply kill the host* they both accomplish the same task. You're task there is to get the job done. The question of the OP covers the best style in which your task gets done.
Evan Carroll
could you provide a resource that shows where/why these command statements are GOTO's? Granted, im more curious about how the label's interact with these statements but if we could state that the labels are merely extensions of these control statements and the control statements are gotos I might be okay with conceding that the use of these labels is on the same level as a goto. I havent quite convinced myself yet though.
stocherilac
Loop control statements are conceptually a lot different from arbitrary jump constructs like goto. In languages where loops are a primary control structure it makes sense to have keywords that specifically aid in building efficient loops.
Matthew S
@Matthew I never said it was an arbitrary goto. It is an instance of a goto. A break is absolutely no different than putting a label just outside the loop and doing a goto to that label. Continue is constructed similarly with a label at the beginning of the loop. If you have no problem using those constructs, you should have no problem using a goto when it is called for.
frankc
@user275455 The syntactic sugar of break, next, last etc. carries with it extra information (the knowledge that it is part of a loop construct) that can be used by the programmer to aid readability and theoretically by the compiler to aid efficiency. The goto keyword is arbitrary because there is no limit on where it can redirect the control flow.
Matthew S
+2  A: 

I would answer it like this, and I'm not sure if this is sufficiently different from what others have said:

Because you can only only move inside of the current scope, or to a parent scope, they're much less dangerous than what is typically implied by goto, observe:

if (1) {
  goto BAR;
  die 'bar'
}
BAR:

This should work obviously, but this won't (can't move in this direction).

if (0) {
  BAR:
  die 'bar'
}
goto BAR;

Many use cases of labels differ from goto in that they're just more explicit variants of core flow control. To make a statement that they're categorically worse would be to imply that:

LOOP: while (1) {
  next LOOP if /foo;
}

is somehow worse than

while (1) {
  next if /foo/;
}

which is simply illogical if you exclude style. But, speaking of style, the latter variant is much easier to read - and it does stop you from having to look up for the properly named label. The reader knows more with next (that you're restarting the loop in the current scope), and that is better.

Let's look at another example

while (1) {

  while (1) {
    last;
  }

  stuff;

}

-vs-

FOO: while (1) {

  BAR: while (1) {
    next FOO;
  }

  stuff;

}

In the latter example here next FOO, skips stuff -- you might desire this, but it is bad idea. It implies that the programmer has read a parent scope to completion which is an assumption probably better avoided. In summary, label isn't as bad as goto and sometimes they can simplify code; but, in most cases they should be avoided. I usually rewrite loops without labels when I encounter them on CPAN.

Evan Carroll
What are your thoughts in terms of readability? Lets say that I do in fact want to skip 'stuff' based on some condition in the BAR loop. Then that would mean I would need to add an additional variable that gets set inside of the BAR loop and read inside of the FOO loop just to let the loop know to skip 'stuff' and finish the entire loop. This gets to be even more of an issue when you have a particularly 'wordy' loop that needs to be maintained. You now have to keep track of logic in multiple location in order to ensure that the logic of skipping 'stuff' is maintained. Thoughts?
stocherilac
I would argue in general setting a variable more typically follows expectation; and, in that, you're much less likely to violate the *principle of least surprise*. There are other ways to skip stuff given to you with universal flow control. If `stuff;` is dependent on the content of the loop - move it inside to the bottom of loop (`BAR`), then promote `next` to `last`. This will end `BAR`, and start the next iteration of `FOO`.
Evan Carroll
Sorry, something you said confuses me and if true would shock me. Putting a last inside of a nested loop causes the outer loop to iterate? i.e. putting last in BAR would cause stuff to get skipped? Maybe I just misunderstood.
stocherilac
No putting a `last` inside of a nested loop at the bottom of an outer loop will though. We we're assuming stuff is dependent on the inner loop, and if it is - it could be placed inside the inner loop., then you place `last` inside of the inner loop, and without any `stuff;` underneath it will naturally roll over to the next iteration of the outer loop. If you do have `stuff;` that remains underneath it - then that executes as expected. `OUTER: while (1) { INNER: while (1) { last; stuff; # will shoot back to the start of next OUTER without executing stuff; } so long as nothings here; }`
Evan Carroll
Consider this case: You have a collection of hosts that make up a cluster. On that cluster you have some VM's. You want to verify that each host that makes up the cluster shares the same networks so that you can confirm that VM's are able to swap from host to host without issue. So you use two loops. First loop goes through host A's networks the second goes through Host B's. If a match is found go to Host A's next network. If no match is found then log. To me this is an idea situation for using loop labels to exit an inner loop. Maybe too simplified, but thoughts?
stocherilac
I'll look at if you can provide code, maybe gist.github.com. Right now though I can do all of that in this `use List::Util 'first'; foreach my $lnet ( $hosta->network ) { warn 'No match' unless first { ip_cmp($rnet,$lnet) } $host-b>network }`
Evan Carroll
+3  A: 

IMO, your code comparison is unfair. The goal is readable code.

To be fair, you should compare an idiomatic Perl nested loop with labels against one without them. The C style for and blocked if statement add noise that make it impossible to compare the approaches.

Labels:

OUTER: for my $wid (@ary1) {
    INNER:   for my $jet (@ary2) {
                 next OUTER if $wid > $jet;
                 $wid += $jet;
             }
       }

Without labels:

for my $wid (@ary1) {
   for my $jet (@ary2) {
       last if $wid > $jet;
       $wid += $jet;
   }
}

I prefer the labeled version because it is explicit about the effect of the condition $wid > $jet. Without labels you need to remember that last operates on the inner loop and that when the inner loop is done, we move to the next item in the outer loop. This isn't exactly rocket-science, but it is real, demonstrable, cognitive overhead. Used correctly, labels make the code more readable.

Update:

stocherilac asked what happens if you have code after the nested loop. It depends on whether you want to skip it based on the inner conditional or always execute it.

If you want to skip the code in the outer loop, the labeled code works as desired.

If you want to be sure it is executed every time, you can use a continue block.

OUTER: for my $wid (@ary1) {
    INNER:   for my $jet (@ary2) {
                 next OUTER if $wid > $jet;
                 $wid += $jet;
             }
       }
       continue {
           # This code will execute when next OUTER is called.
       }
daotoad
what happens when you have code AFTER the nested loop?
stocherilac
right, but then if you do use `last` you skip `continue`, and you're back in the land of needlessly complex. To revisit your statement *"I prefer the labeled version because it is explicit"*... It isn't really explicit about the potentially skipped code. This isn't so with `last`, which means simply explicitly *I'm done here (this scope)*. It feels like it is wrong in the same sense as global variables or `exit` is wrong. If there is no need to explicitly direct the operations of your parent scope -- why do it? Just say your done and let it happen naturally.
Evan Carroll