using a subroutine

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

Hello Monks,

I have been learning perl for the last month or so. Having gone through several tutorials writing practice code I decided it was time to try one of my own.

Following is some code that works for removing emacs tmp files and core files generated by the kernel from the /home/john and /lib directories. One of my goals for this is to easily change which directories are parsed.

My question is on the way I setup the parameters to be passed to the subroutine. Is this the correct way to setup multiple arguments for the subroutine?

#!/usr/bin/perl -w

$user = "/home/john";
$lib = "/lib";

&directs($lib);
&directs($user);

sub directs {
    foreach (@_) {
        opendir(DIRS,$_);
        @tmps = readdir(DIRS);

        closedir(DIRS);
    }
    &remove;
}

sub remove {
    foreach $line(@tmps) {
    if (($line =~ /~$/ ) || ($line =~ /^core\./)) {
    unlink "$line";
        #print "$line\n";
    }
  }
}
[download]

All of these are really good answers. I will start implementing (or learning how to use them) immediately.

One of the reasons that I made this post is to learn the correct way and get into the habit of doing things that way. Fortunately I don't have any bad habits to break since I have never programmed before-I am a network administrator by profession. All of you have supplied me with good habits to get into.

Thank you very much!

John

Comment on using a subroutine Download Code

Replies are listed 'Best First'.
Re: using a subroutine by bgreenlee (Friar) on Aug 08, 2004 at 22:23 UTC
Good start. Some suggestions: Use 'my' when declaring your variables. Global variables are generally Not Good Assuming those aren't the only two directories you'll ever want to process, consider passing in the directories on the command line, either directly through @ARGV, or via Getopt::Long (or Getopt::Std, if you're old-school) Congrats on using -w...now use strict in this program, there's really no reason to have the directories read by one subroutine and processed by another. A subroutine should generally be potentially useful to more than one caller. If it is only ever going to be called in one place, consider just moving it to that place (one exception to this "rule" is if you're breaking out significant chunks of code for increased clarity) Brad	[reply]
Re^2: using a subroutine by xjlittle (Beadle) on Aug 10, 2004 at 00:32 UTC
Brad, your points are well taken. I will get into the habit of watching my `my`'s and my `()`'s. Thanks! John	[reply] [d/l] [select]
Re: using a subroutine by ysth (Canon) on Aug 08, 2004 at 22:42 UTC
`sub directs { ... &remove; }` [download] When calling subroutines using the &, you should almost always use () also: `&remove();`. Not doing so gives the called routine access to the caller's @_; that is, a shift() in remove would take away one of direct's arguments. This can lead to hard-to-find bugs. &subname; without parens is only useful as an optimization in rare circumstances. Alternatively, drop the & too, and either place the body of your subs before they are called, or use a forward declaration: `sub remove; sub declare;`	[reply] [d/l] [select]
Re^2: using a subroutine by beable (Friar) on Aug 08, 2004 at 22:57 UTC
Why not just place your subs whereever you like, call them from whereever you like without & and with (), and not use forward declarations either? What am I missing? Perl isn't C. This seems to work for me: Read more... (678 Bytes)	[reply] [d/l]
Re^3: using a subroutine by ysth (Canon) on Aug 08, 2004 at 23:15 UTC
I agree, but if you are in the habit of leaving off (), it may be easiest to predeclare.	[reply]
Re^2: using a subroutine by xjlittle (Beadle) on Aug 10, 2004 at 00:34 UTC
Ditto, same as Brad ^	[reply]
Re: using a subroutine by Zaxo (Archbishop) on Aug 09, 2004 at 06:14 UTC
One thing you do which strict may call attention to is set a global variable in one subroutine and use it in another. It's better to just pass those values as arguments. You'll wind up with cleaner code that way. You can also save yourself some trouble by realizing that unlink can take a list as argument, and that glob will produce a list of the kind you want. Here is a sub that illustrates a more economical approach. `my ($user, $lib) = qw(/home/john /lib); sub directs { my $count; for my $dir (@_) { $count += unlink map {glob "$dir/$_"} qw(~ core.); } $count; } print directs( $user, $lib), ' files deleted.', $/;` [download] The map applies the code in its first argument to each element of the remaining arguments - here, globby versions of your regexen. The result is a list of matching filenames. unlink then acts on each, returning the number of files deleted in that pass. Update: $/ is the input record seperator, by default your system's newline. You could say that as well with `"\n"`. Using $/ is handy if you might want to change the line end, because then you can say, for instance, `{ local $/ = '<br />'; foo(@args); }` [download] and the strings foo() prints will be terminated with html break tags. `$\` could also be used (with even better justification) but it does not have the newline default value. After Compline, Zaxo	[reply] [d/l] [select]
Re^2: using a subroutine by xjlittle (Beadle) on Aug 10, 2004 at 00:50 UTC
Zaxo, One other question after studying your code. On the last line: `print directs( $user, $lib), ' files deleted.', $/;` [download] what does the `$/;` do? Thanks, John	[reply] [d/l] [select]
Re^2: using a subroutine by xjlittle (Beadle) on Aug 10, 2004 at 00:28 UTC
This worked beautifully! Now all I need is to understand the map command better. I haven't seen that before. Thanks for the link and the advice! I do have a question though. When I first set this up I had a comma between (/home/john /lib) in the first line of code: `my ($user, $lib) = qw(/home/john /lib);` [download] which caused it to not work. Why does the comma have this affect on this or any other script? John	[reply] [d/l]
Re: using a subroutine by bradcathey (Prior) on Aug 09, 2004 at 00:31 UTC
Nice work. And yes, `use strict` is a real life-saver. I guess your next lesson will be passing arrays to and from subs—and the mandatory introduction to references, something that took me a while to get my head around. Checkout Randal L Schwartz's Learning Perl Objects, References & Modules or the Perl Cookbook. Good luck. —Brad "Don't ever take a fence down until you know the reason it was put up. " G. K. Chesterton	[reply] [d/l]
Re^2: using a subroutine by xjlittle (Beadle) on Aug 10, 2004 at 00:42 UTC
Yep, it will be. I've been using one of Schwartze's books on Learning Perl which gives a brief but solid introduction into many aspects of using Perl-with some good end of chapter test questions to help the learning process. Passing anything to a sub, much less out of it, still causes me a lot of study to understand what I need to do :-). I'll certainly look into the Learning Perl Objects...book. Thanks for the help! John	[reply]
Re: using a subroutine by Jasper (Chaplain) on Aug 09, 2004 at 08:36 UTC
Your call to remove will only ever remove the files from the last directory you read. `@tmps = readdir(DIRS);` should probably be `push @tmps, readdir(DIRS);`. I second everyone else's comments about globals, etc..	[reply] [d/l] [select]
Re^2: using a subroutine by xjlittle (Beadle) on Aug 10, 2004 at 00:05 UTC
Oops you're correct. Using `push @tmps,readdirs(DIRS)` didn't delete the tmp files either. Back to the drawing board...	[reply] [d/l]
Re: using a subroutine by mpeg4codec (Pilgrim) on Aug 09, 2004 at 17:58 UTC
Looking very good! It's a little redundant to pass the two arguments to `directs()` separately. You have the right idea with the `foreach (@_)` in the subroutine. Just pass the options like this: `&directs($home, $lib);` [download] Remember, the rule for learning how to program is ``Read a lot, code a little, read a lot, code a little.'' Always keep experimenting. Good luck!	[reply] [d/l]
Re^2: using a subroutine by xjlittle (Beadle) on Aug 09, 2004 at 23:51 UTC
I tried this way and it only read the last argument, $lib. However I may very well have missed something in the code that would cause both to be read. I like your philosophy though!	[reply]
Re: using a subroutine by trammell (Priest) on Aug 09, 2004 at 16:52 UTC
Perl is a great language, but sometimes the system utilities are a better tool, e.g. (untested): `find /lib -name 'core.*' -exec rm -f {} \;` [download]	[reply] [d/l]
Re^2: using a subroutine by xjlittle (Beadle) on Aug 09, 2004 at 23:53 UTC
You're absolutely right it would work. However I've been writing bash scripts for several years and this is not my goal-perl is. Thanks for taking the time to write!	[reply]
Re: using a subroutine by TomDLux (Vicar) on Aug 10, 2004 at 01:52 UTC
I would characterize a subroutine as a self-contained set of operations, performed on the arguments passed to it. opendirs sets the variable filehandle specified in the first argument, setting it to access the file system directory specified in the second argmument. unlink removes the file specified in its argument. rand(2) does something, and rand(100) does something similar but different, dependent on the different arguments. In particular, routines should be testable in isolation or in small groups. So while you're doing well to use subroutines, if they are dependent on global variables then you're really writing one global, interconnected set of code. Addmittedly, in certain circumstances you may have global configuration variables, our $PI=3.14; or our $DEFAULT_SEPARATOR = ','; but that's a slightly different case .... but do try to avoid even those if you can. As far as the list assignment: `my ($user, $lib) = qw(/home/john /lib);` [download] is concerned, it works perfectly well: `my ($user, $lib) = qw(/home/john /lib); + print "\$user is '$user'.\n"; print "\$lib is '$lib'.\n"; # # Generates the output: # $user is '/home/john'. $lib is '/lib'.` [download] -- `TTTATCGGTCGTTATATAGATGTTTGCA`	[reply] [d/l] [select]

Back to Seekers of Perl Wisdom