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

fellow monks-
i am writing this query as more of a "did i do this correctly?" rather than a "how do i...?".
the code i am working with involved a fairly lengthy subroutine, which i have since broken into two subs as the newest revision of the code added too much and made readability taxing.
however, i now find that a variable called in the first routine, and declared with my $error is to be used in the second routine. since i am using use strict; i receive a complaint that $error in the second routine needs to be declared. the snippet of my code is:

sub exec { ... ## retrieve an error message if unsuccessful my $error = $s->error if ($count > 0); ## log this action &log_action('error') if ($count > 0); ... }

now the log_action subroutine has the following line:

sub log_action { my $i = shift; if ($i eq "error") { print "\n$error\n\n"; print FH "$format $error"; ... }

i fixed this by just removing the my localization for $error outside of the subroutine to a global statement of my $error; earlier in the script. however, i just dont like the look of this declaration standing all by its lonesome. is there another/better method of passing the value of $error from subroutine to subroutine?

humbly -c

Replies are listed 'Best First'.
Re: passing a variable from one subroutine to another
by Zaxo (Archbishop) on Oct 05, 2001 at 14:44 UTC

    The name exec is poorly chosen for a perl sub. See perldoc -f exec.

    The preferred way of passing an error value along is to do so directly:

    #from 'exec' sub &log_action($error) if ($count > 0); # ... #later... sub log_action { my $error = shift; print "\n$error\n\n" && # next print FH is fishy. When is FH opened and closed? print FH "$format $error" if $error; }
    It is bad design to use globals or widely scoped lexicals to pass values to subs.

    After Compline,
    Zaxo

Re: passing a variable from one subroutine to another
by C-Keen (Monk) on Oct 05, 2001 at 14:18 UTC
    A possible way to do so is to pass $error on to log_action a second parameter. That would be:
    sub exec{ #... &log_action('error',$error); } sub log_action{ my $i = shift; #get first parameter my $error = shift; get second parameter # note that this $error here is not the same variable but has the same + value! #do whatever with it... }

    You should also consult your Camel Book on Subroutines.

    Hope this helps,
    C-Keen

Re: passing a variable from one subroutine to another
by jeroenes (Priest) on Oct 05, 2001 at 14:38 UTC
    You are asking about general advice, so that's what you'll get ;-).

    What you have done (implicitly) is creating a global variable. That's a Bad Thing (tm).

    Why? Because your subs become dependent on each other. On its turn that means that your code becomes harder to debug/ maintain, harder to reuse ("Where the hack does that $error come from?") and easier to break, among others.

    So, pass the variable to your sub. Or pass an object or hash that contains the info or has the right methods. Everything is better than a global.

    Try to read 'Code Complete' by McConnell to get it explained better than I could....

    Jeroen
    "We are not alone"(FZ)

    PS: ++ for the use of strict, and for your approach to break the sub into components. Maybe try to think of the different functionalities. Like: Is error reporting a subfunction of logging, or something separate, or at another level, or something that the average user should be protected from (yes!)?

Re: passing a variable from one subroutine to another
by blackjudas (Pilgrim) on Oct 05, 2001 at 20:32 UTC
    I'll agree with Zaxo on the fact that you shouldn't be using reserved words as variables/sub names. As far as using local to scope a global, most people recommend against using it since it can confuse the flow of a program. Though using it is fully valid if you want to make your $error a global. I would suggest the following solution:
    sub execute { my $error = $s->$error if ($count > 0); ## log this action &log_action($error) if ($count > 0); # just pass the $error var to &log_action } sub log_action { my $i = shift; if ($i) { print "\n$i\n\n"; print FH "$format $error"; } }
    Hope that does the trick for you.

    BlackJudas
Re: passing a variable from one subroutine to another
by jerrygarciuh (Curate) on Oct 05, 2001 at 17:52 UTC
    First let me say that this advice comes from a neophyte, but I believe that in Chapter 8 of the Llama ...looks... yeah, on page 98 if you have it, you can make a variable available to any subs called from a sub by declaring it with local instead of my. So If you call the second sub from the first and declare with local, you should be able to share the value without going global.
    hth
    jg
    _____________________________________________________
    Ain't no time to hate! Barely time to wait! ~RH
      It's still "global", or more accurately, "a fine upstanding resident of the symbol table". local doesn't work on lexical (declared with my) variables.

      local simply hides away the existing value for the duration of the lexical scope. It's still accessable through the symbol table (as a global), but within the scope, including in any subroutines called from the scope, it has a temporarily new value. For more practical information, scope out Seven useful uses of local by a local octopus enthusiast.

      There, that ought to protect you from merlyn!

        I really appreciate the links here, in following them I also found What's the difference between dynamic and lexical (static) scoping? Between local() and my()? to be very helpful.

        In reading the aforementioned passage in Llama (p98) again, it seems to me that adherance to the strict pragma is more of a PM thing. I get the feeling that, since merlyn addresses it briefly at Chapter 8 of Llama 2nd ed., the message of the Llama is that it is more efficacious not to worry about strict when beginning to learn perl. Comments? Trout slaps from Camel ot?
        TIA
        jg
        _____________________________________________________
        Ain't no time to hate! Barely time to wait! ~RH
Re: passing a variable from one subroutine to another
by dragonchild (Archbishop) on Oct 08, 2001 at 18:06 UTC
    This sounds like you're doing some sort of error-logging and reporting. There are a number of good ways to do this, many of them on CPAN. I'd go take a look there, first.

    Secondly, a few general coding suggestions.

    my $error = $s->error if ($count > 0); ## log this action &log_action('error') if ($count > 0);
    This would be better written as
    if ($count > 0) { my $error = $s->error; &log_action('error', $error); }
    This way, it's obvious that the two actions take place at the same time. Now, if you never need to use $error again, I would reccomend doing something like
    &log_action('error', $s->error) if ($count > 0);
    Another thing - your variable names need improvement. Right now, you know what $s and $i do. But, will you in six months? Maybe rewrite log_action() as such
    sub log_action { my ($action_type, $string) = @_; if (lc($action_type) eq 'error') { print "\n$string\n\n"; print FH "$format $string\n"; ... }
    In addition, I would look at passing the filehandle into the log_action() sub-routine. Maybe, use IO::File to create a reference to a filehandle and pass it around. This way, log_action() knows as little as possible about its world, making it possible to move it around to new scripts you might write. (Maybe, you might even look at putting log_action() into its own module, with a few other error-logging routines?)

    ------
    We are the carpenters and bricklayers of the Information Age.

    Don't go borrowing trouble. For programmers, this means Worry only about what you need to implement.