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

Oh good monkies, I need thy wisdom. I'm doing a data integrity check on data I'm receiving from a DB (that I can't control) and if an integrity problem is found, I run a sub that takes care of a few things like printing to the stdOut log, creating an error message and throwing it into an array for later emailing to our team, AND (here's the big one) ... calling NEXT to jump out of the current foreach loop (I'm foreaching through each row of my dataset. Code looks something like this:
my @problems; foreach my $row (@$data) { my ($cusip, $isin, $sedol, $ticker) = @$row; &problem("Missing cusip for $ticker") unless ($cusip); &problem("Missing isin for $ticker") unless ($isin); &problem("Missing sedol for $ticker") unless ($sedol); } sub problem { my $issue = shift; print "$issue\n"; push (@problems, $issue); next; }

Now, this code works correctly but throws the old "Exiting subroutine via next at <line>" junk to stdErr and I don't like that. I know that I could pull the "next" out of the sub and write my if/unless like this:

if !($cusip) { &problem("Missing cusip for $ticker"); next; }
But that's going to get ugly considering I've got about a dozen pieces of data to check and I hate using up 60 lines of code just to do the integrity checks. Any ideas? --Billy

Replies are listed 'Best First'.
Re: Exiting subroutine via next -- Again
by ikegami (Patriarch) on Jul 13, 2011 at 20:56 UTC

    Check for validity inside a sub.

    my @problems; sub required { my ($val, $msg) = @_; if ($val) { return 1; } else { push @problems, $msg; return 0; } } for my $row (@$data) { my ($cusip, $isin, $sedol, $ticker) = @$row; required($cusip, "Missing cusip for $ticker") or next; required($isin, "Missing isin for $ticker") or next; required($sedol, "Missing sedol for $ticker") or next; }

    Update: The condition was inverted. Fixed.

      Simple and brilliant. I bow to thee monk.
Re: Exiting subroutine via next -- Again
by davido (Cardinal) on Jul 13, 2011 at 21:27 UTC

    Putting aside the problem for a moment, In your case, I think your code would become clearer and easier to maintain by not modifying @problems from a distance inside the subroutine. But feel free to change my example back to using global osmosis as a means of returning value from a subroutine if you think it is better in your case. (99% of the rant removed) :)

    This example uses a hash of categories, where the categories are cuspid, isin, sedol, and ticker. You can iterate over each category and test it for compliance or problems.

    Try this:

    my @problems; foreach my $row ( @$data ) { my %categories; @categories{ cusip isin sedol ticker } = @$row; foreach my $category ( keys %categories ) { next if $category eq 'ticker'; if( not defined( $categories{$category} ) { push @problems, problem( $category, $categories{ticker} ); next; } } } sub problem { my ( $problem_tag, $ticker ) = @_; my $issue = "Missing $problem_tag for $ticker"; print $issue, "\n"; return $issue; }

    In other words, if you have all that data to check, don't use individual variables, use a hash so that you can easily iterate over the keys (and not be tempted to resort to symbolic ref manipulation). The sub is defined outside the scope of your loop, and the next would have no idea where to next to. The loop isn't part of the caller stack. next has to come from inside the loop. Also, no need to use the &functionname notation in Perl5 under this circumstance. Try to get into the habit of not using it. Finally, wouldn't you agree it's better to not have a sub modifying a globally scoped variable when it can be avoided?


    Dave

Re: Exiting subroutine via next -- Again
by choroba (Cardinal) on Jul 14, 2011 at 08:28 UTC
    Oh good monkies, I need thy wisdom.
    "Thy" is singular, "monkies" is plural. Use "monkies" with "your", or "monkey" with "thy" :)
Re: Exiting subroutine via next -- Again
by JavaFan (Canon) on Jul 14, 2011 at 09:07 UTC
    Now, this code works correctly but throws the old "Exiting subroutine via next at <line>" junk to stdErr and I don't like that.
    Why not do the obvious, and turn off the warning? The warning is Perl saying "You're doing X, are you sure?". If you are sure (and you are), you ought to turn off the warning. Changing you code backwards just to avoid the warning shouts to anyone reading your code that you do not understand warnings.

    Remember, warnings are there for the programmer. They are a tool, nothing more, nothing less. Do not become a slave of warnings.