Beefy Boxes and Bandwidth Generously Provided by pair Networks
Keep It Simple, Stupid
 
PerlMonks  

Re: Subroutine and variable passing

by kcott (Archbishop)
on Aug 24, 2013 at 11:22 UTC ( [id://1050806]=note: print w/replies, xml ) Need Help??


in reply to Subroutine and variable passing

G'day tman77,

Welcome to the monastery.

Had you been using strict, you would've received a strict vars error explaining the problem. ${symbol} is neither declared nor defined so $wget_url evaluates to:

"/home/user/sym_dir/_2013 'http://www.downloadsite.com/.dat'"

Had you declared, but not defined, ${symbol}, Perl would've warned you of that as well, if you'd been using warnings. So, unless you have a very good reason not to, always put

use strict; use wanings;

at the start of your code. Typing those few characters and being advised immediately by Perl of problems with your code is obviously a lot less work than having to post a question in a forum; and, also obviously, the feedback is a lot quicker.

I don't think your approach to this is a particularly good one; although, without knowing more about the context of your complete code, it's difficult to specifically advise what would be better. I'd probably have used some sort of token in place of ${symbol} and substituted that with the symbols from the file; something along these lines:

$ perl -Mstrict -Mwarnings -le ' my $wget_url = "/some/path/_SYMBOL__2013 http://example.com/_SYMBO +L_.dat"; my @symbols = qw{A B C}; for (@symbols) { (my $sym_url = $wget_url) =~ s/_SYMBOL_/$_/g; print $sym_url; } ' /some/path/A_2013 http://example.com/A.dat /some/path/B_2013 http://example.com/B.dat /some/path/C_2013 http://example.com/C.dat

A few other issues with your code:

  • Don't prepend an ampersand ("&") to your function names unless you have a good reason for doing so and understand what that reason is. Adding an "&" is possibly not doing what you think it is and may have unexpected side-effects. See "perlsub - Perl subroutines" for more details.
  • As it doesn't look like @sym_array changes, move the I/O code (that reads its values) out of the get_symbol_data subroutine: you don't need to populate @sym_array on every call to get_symbol_data().
  • Don't terminate your die message with a newline. Perl appends file and line information to your message (that you normally want) as well a newline anyway. Adding your own newline stops this from happening. See the die documentation for details.
  • It's better to use the 3-argument form of open. See the open documentation for usage examples and reasons for doing this.
  • Avoid working directly with $_[0], $_[1], etc in subroutines. You run the risk of altering the original data passed to the subroutine, possibly in subtle and unexpected ways that result in bugs that are hard to track down. Read arguments preferably like this "my ($x, $y, $z) = @_;"; while you can use "my $x = shift;" for single arguments, subroutines have a habit of requiring additional arguments as they mature and you end up with this common bug "my ($x, $y) = shift;", which doesn't happen if your wrote "my ($x) = @_;" in the first place.

-- Ken

Log In?
Username:
Password:

What's my password?
Create A New User
Domain Nodelet?
Node Status?
node history
Node Type: note [id://1050806]
help
Chatterbox?
and the web crawler heard nothing...

How do I use this?Last hourOther CB clients
Other Users?
Others browsing the Monastery: (8)
As of 2024-03-28 11:12 GMT
Sections?
Information?
Find Nodes?
Leftovers?
    Voting Booth?

    No recent polls found