Beefy Boxes and Bandwidth Generously Provided by pair Networks
Think about Loose Coupling
 
PerlMonks  

using a subroutine

by xjlittle (Beadle)
on Aug 08, 2004 at 21:47 UTC ( [id://381112]=perlquestion: print w/replies, xml ) Need Help??

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"; } } }


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

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

      Brad,
      your points are well taken. I will get into the habit of watching my my's and my ()'s.

      Thanks!
      John
Re: using a subroutine
by ysth (Canon) on Aug 08, 2004 at 22:42 UTC
    sub directs { ... &remove; }
    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;

      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:
        I agree, but if you are in the habit of leaving off (), it may be easiest to predeclare.
      Ditto, same as Brad ^
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.', $/;
    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); }
    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

      Zaxo,

      One other question after studying your code. On the last line:
      print directs( $user, $lib), ' files deleted.', $/;
      what does the $/; do?

      Thanks,

      John
      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);

      which caused it to not work. Why does the comma have this affect on this or any other script?

      John
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
      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
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..
      Oops you're correct. Using
       push @tmps,readdirs(DIRS)
      didn't delete the tmp files either.
      Back to the drawing board...
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);
    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!
      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!
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 {} \;
      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!
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);
    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'.

    --
    TTTATCGGTCGTTATATAGATGTTTGCA

Log In?
Username:
Password:

What's my password?
Create A New User
Domain Nodelet?
Node Status?
node history
Node Type: perlquestion [id://381112]
Approved by ysth
Front-paged by beable
help
Chatterbox?
and the web crawler heard nothing...

How do I use this?Last hourOther CB clients
Other Users?
Others examining the Monastery: (4)
As of 2024-04-25 08:39 GMT
Sections?
Information?
Find Nodes?
Leftovers?
    Voting Booth?

    No recent polls found