Win has asked for the wisdom of the Perl Monks concerning the following question:

Dear Monks,

I have used a goto statement in the following bit of code. Is there a better way than this? <background>
#! perl -w use strict; use warnings; my @files; my @List_of_commands; my $line; my $file; while (<*.sql>) { next if $_ eq 'Activate.sql'; push (@files, $_); } my $line_number; foreach (@files) { print $_; $file = $_; open (FILE, "<$file"); $line_number = 0; while (<FILE>){ my $line = $_; $line_number = $line_number +1; if ($line_number == 1){ if ($line !~ m/ALTER\sPROCEDURE\s/){ #do nothing } else { goto here; } } $line = $_; if ($line =~ m/INSERT INTO Message_pool/){ #Do nothing } elsif ($line =~ m/CREATE TABLE\s([A-Z|a-z|_]{1,200})\s.{0,100}/) { my $table_name = $1; my $Command = "if exists (select 1 from INFORMATION_SCHEMA.table +s where table_name = '".$table_name."') DROP TABLE ".$table_name.""; push(@List_of_commands, $Command); } else { #Do nothing } } here: } my $outfile = "Outfile.txt"; foreach (@List_of_commands){ open (OUTFILE, "+>>$outfile"); print OUTFILE "$_\n" }

Replies are listed 'Best First'.
Re: GOTO statement. A better way?
by BrowserUk (Patriarch) on Sep 19, 2005 at 09:38 UTC

    Your goto can be replaced by last


    Examine what is said, not who speaks -- Silence betokens consent -- Love the truth but pardon error.
    Lingua non convalesco, consenesco et abolesco. -- Rule 1 has a caveat! -- Who broke the cabal?
    "Science is about questioning the status quo. Questioning authority".
    The "good enough" maybe good enough for the now, and perfection maybe unobtainable, but that should not preclude us from striving for perfection, when time, circumstance or desire allow.
Re: GOTO statement. A better way?
by Corion (Patriarch) on Sep 19, 2005 at 09:26 UTC

    First of all, did you see the preview option and use it? Posting your code twice in one node doesn't improve the readability or the understanding.

    Second, your question is easily answered by perldoc -f next.

    A reply falls below the community's threshold of quality. You may see it by logging in.
Re: GOTO statement. A better way?
by sh1tn (Priest) on Sep 19, 2005 at 10:37 UTC
Re: GOTO statement. A better way?
by tilly (Archbishop) on Sep 20, 2005 at 09:03 UTC
    Untested, off of the top of my head, here is a cleaner version of the above:
    #! perl -w use strict; use warnings; my $outfile = "Outfile.txt"; open (OUT, "+>>$outfile") or die "Cannot append to '$outfile': $!"; FILE: while (<*.sql>) { my $file = $_; next FILE if $file eq 'Activate.sql'; open (my $fh, "<", $file) or die "Cannot read '$file': $!"; print "$file\n"; while (<$fh>) { if (1 == $. and m/ALTER\sPROCEDURE\s/) { next FILE; } if ( not m/INSERT INTO Message_pool/ and m/CREATE TABLE\s([A-Z|a-z|_]{1,200})\s.{0,100}/ ) { print OUT "if exists (select 1 from INFORMATION_SCHEMA.tables wh +ere table_name = '$1') DROP TABLE $1\n"; } } }
    Here is a list of what I changed:
    1. Declare variables close to where they are first used.
    2. Avoid looping over the same set multiple times.
    3. Be willing to use loop control.
    4. Have error checks as we are told in perlstyle.
    5. Use 3 arg open (see Two-arg open() considered dangerous for why).
    6. If you're going to use $_, use $_.
    7. Being willing to use $. allows you to avoid maintaining your own index.
    8. It is better to say (1 == $var) rather than ($var == 1) because then forgetting an = in there results in an illegal instruction rather than a nasty logic bug.
    9. Avoid having lots of empty cases on your conditionals just to say "do nothing". It should be obvious that missing cases do nothing.
    10. Interpolation is good. Use it.
    11. Don't build up a data structure just to print it later, print it the first time!
    12. I added "\n" where it seemed appropriate.

    UPDATE: hv pointed out that I was still using $table_name when the data was in $1. Fixed.

Re: GOTO statement. A better way?
by AReed (Pilgrim) on Sep 19, 2005 at 22:25 UTC
    As a matter of style, unnecessary negatives in conditional statements make code harder to read. If "#do nothing" really does mean that there's nothing to do, then this:
    if ($line !~ m/ALTER\sPROCEDURE\s/){ #do nothing } else { goto here; }
    would be more easily understood if written as:
    if ($line =~ m/ALTER\sPROCEDURE\s/) { last; }