Re: Ghostly subroutine variables haunting me in subsequent calls
by davido (Cardinal) on Oct 25, 2005 at 16:50 UTC
|
The array @pileofinchis is not declared as a lexical within the subroutine that you displayed to us. If it's safe for me to infer that variable is declared at a broader scope (or is a package global), whatever got pushed into it in the first call to the subroutine will still be there when you call the sub the second time and each subsequent time.
In my opinion, one of the worst things you can do with respect to subroutines is allow them to accept values through broader-scope "osmosis". My opinion is that all subroutine variables should be lexically scoped to the boundries of the subroutine. There are exceptions; closures for example. But you have to be careful whenever a value is passed to a sub via broader scope instead of via parameter list.
| [reply] [d/l] |
|
|
In my opinion, one of the worst things you can do with respect to subroutines is allow them to accept values through broader-scope "osmosis". My opinion is that all subroutine variables should be lexically scoped to the boundries of the subroutine.
Huh?!? That's exactly what a closure does. And closures are useful!
Update: I hadn't noticed you mention closures yourself. I apologize. But in any case some caution is required IMHO before stating such a claim as the above. I.e. I wouldn't talk about "one of the worst things you can do...", but of "you must pay a lot of attention when you...", instead.
| [reply] |
|
|
People often create closures by accident, not realizing that they work this way. I see it a few times a week on the mod_perl list. Closures may have uses, but they are clearly a major source of bugs in perl code.
| [reply] |
|
|
|
|
Presumably, if you are creating a closure for a useful purpose, you already know davido's advice doesn't apply. I agree with him† that it is "one of the worst things you can do..." unless, of course, you happen to be doing it on purpose. I don't think he really needs to include that caveat... Do you?
† Well, one of the worse in terms of creating hard to find bugs in your Perl programs, anyway. It's not like homicide or something... (-:
-sauoq
"My two cents aren't worth a dime.";
| [reply] |
|
|
I don't understand how you could take issue with my statement if you read the entire short paragraph, including this part:
There are exceptions; closures for example. But you have to be careful whenever a value is passed to a sub via broader scope instead of via parameter list.
How could you possibly name closures as an example of how my assertion is outlandish if I, in the very same paragraph named closures as one exception? I even acknowleged that there are other exceptions too. You did read the entire paragraph right? Or should I have enumerated all exceptions within the same sentence as the assertion?
| [reply] |
|
|
In response to all this lively discourse, the problem continues in spite of a suitably scoped my @pileofinchis array. However, I have now found at least one problem which was indeed a scoping issue with an array.
use strict scares me :(
Thanks for your assistance
| [reply] [d/l] |
|
|
use strict scares me
Disuse of strictures scares me, at least when its absence is unaccompanied by a darn good reason.
A few brief lines of code is one thing. But if you're serious about scripting in Perl, you'll need to learn to live within the criteria set by use strict;
As for your problem at hand, here's my recommendation. Boil your script and subroutine down to a minimal amount of code that will actually compile and run in such a way as to demonstrate to us the problem. Cut and paste it into a followup in this thread so that we can look it over. We need to see actual code that actually runs, that actually replicates the problem, and it needs to be under 30 lines of code. Chances are good that while you work at creating a test example you'll end up discovering the problem on your own anyway, but if not, post it and we'll help further.
| [reply] [d/l] [select] |
|
|
In response to all this lively discourse, the problem continues in spite of a suitably scoped my @pileofinchis array.
It may not be the actual problem, but as I already wrote in my reply to BrowserUk's one, I'd be curious to know what you are doing with that reference of it that you return from your sub.
use strict scares me
Not only, just like davido "disuse of strictures scares me", but your being scared of use strict; scares me too, since there's really nothing to be scared of, that is: in letting perl help you to write good code.
| [reply] [d/l] |
Re: Ghostly subroutine variables haunting me in subsequent calls
by BrowserUk (Patriarch) on Oct 25, 2005 at 16:56 UTC
|
Add use strict;to your code and it will tell you why.
Specifically, the array that you use to return the results of your function to the caller
@pileofinchis is never declared lexically (with my), and so you are (re-)using the same global array each time you call the function. As you only ever add values to the array
push @pileofinchis,$localinchi;
The contents of the array are added to, but never removed, each time you call it.
use strict catches these things for you and saves much time.
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".
In the absence of evidence, opinion is indistinguishable from prejudice.
| [reply] [d/l] [select] |
|
|
Add use strict; to your code and it will tell you why.
This is not 100% sure: @pileofinchis may be declared in a lexical scope enclosing the sub definition. In which case he may even desire to actually push into it. But then I see no reason to return a reference to it as well, and I'd be curious to know what he is doing with it...
| [reply] [d/l] [select] |
|
|
True, but if the OP is deliberately creating a closure, then he probably wouldn't be asking the question.
Even then, use strict (or maybe -w, I use both so I'm never quite sure which is giving me the information), would still be a good idea as it might well warn him of an even deeper and more obscure problem in the event that the closure is not knowingly deliberate:
sub outer{
my @a;
sub inner{
push @a, 1;
return \@a;
}
return 1
};;
Variable "@a" will not stay shared at (eval 3) line 1, <STDIN> line 1.
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".
In the absence of evidence, opinion is indistinguishable from prejudice.
| [reply] [d/l] |
Re: Ghostly subroutine variables haunting me in subsequent calls
by blazar (Canon) on Oct 25, 2005 at 16:50 UTC
|
sub complete_inchi_parse
{
my $inchi = shift;
my @inchibits = split(@/@,$inchi);
Whatever problem you're really experiencing, it seems hard to believe that "this routine works the first time". Indeed it would be sensible to at least post code that compiles. What do you mean that @/@ to really be?!?
More on topic wrt your actual question, I see that you both push into a @pileofinchis variable that's not lexically scoped to your sub and return a reference to it. There's something strange in this modus operandi. I would probably gather a "local" @pile and return it, pushing the return value of this sub into a suitably "global" @pileofinchis.
HTH
| [reply] [d/l] [select] |
|
|
Whoops, had a brain failure there. I was trying to make a /\// clearer for your ease of reading in the manner of a normal regex which is of course inappropriate in a split call. Corrected.
| [reply] |
|
|
You still can use alternate delimiters "in the manner of a normal regex". But you have to be explicit about the operator:
$ perl -le 'print for split m@/@, "foo/bar/baz"'
foo
bar
baz
| [reply] [d/l] |