in reply to Using File::Find with a bit of elegance

Here is a 'straight script' version. Personally, for scripts this small, i really prefer this style (Perl Cookbook recipe 9.7.)
use strict; use File::Find; my $cod = shift or die 'some error message'; my $dbpath = '/path/to/somewhere'; my @files; @ARGV = '.' unless @ARGV; find sub { push(@files, $_) if /\.mp3\z/; }, @ARGV; open(DB, ">$dbpath/$cod.txt") || die $!; print DB map { s/\.mp3\z/\n/; $_ } sort @files;
The need for our has been eliminated because there are no subroutines to share variables across scope (which, IMHO, is a bad idea and an abuse of our).

Looks like the original author was trying to encapsulate some functionality. I question this approach, because the subroutine indexfiles handles too many tasks and while it does accept arguments, indexfiles does not return anything and instead relies on a shared variable. (peruse the search results at google.com for tight coupling loose cohesion for some good reading.)

Like i said before, i personally prefer a script like this to be consise and without unnecessary subroutines. But, since we are on the topic, here is another version that uses two subroutines (not including the File::Find magic):

use strict; use File::Find; my $cod = shift or die 'some error message'; my $dbpath = '/path/to/somewhere'; @ARGV = '.' unless @ARGV; my @files = get_files(@ARGV); open(DB, ">$dbpath/$cod.txt") || die $!; print DB trim(@files); sub get_files { my @file; find sub { push(@file, $_) if /\.mp3\z/; }, @_; return @file; } sub trim { return map { s/\.mp3\z/\n/; $_ } sort @_; }
Add some error checking and you have some nice subroutines. The important thing to remember here is that a subroutine should be as general as possible, only handle as small of task as possible, and act like a mathematical function: input and output.

Good luck :)

jeffa

L-LL-L--L-LL-L--L-LL-L--
-R--R-RR-R--R-RR-R--R-RR
B--B--B--B--B--B--B--B--
H---H---H---H---H---H---
(the triplet paradiddle with high-hat)

Replies are listed 'Best First'.
(jeffa) 2Re: Using File::Find with a bit of elegance
by jeffa (Bishop) on Mar 31, 2002 at 22:57 UTC
    I preach about separation of presentation and content when i discuss CGI scripts, but i unintentionally strayed from that principle with this subroutine:
    sub trim { return map { s/\.mp3\z/\n/; $_ } sort @_; }
    How? By 'decorating' each filename in the list with a newline. Oops!

    Better would be the following:

    sub trim { return map { s/\.mp3\z//; $_ } sort @_; } # and decorate it in the 'client' print DB map {"$_\n"} trim(@files);
    Of course, the downside is two maps instead of one, but, this is better when making generic, robust, reusable subroutines. Allowing the client to make this decision could very well cut back on the number of modifications required for that subroutine.

    jeffa

    L-LL-L--L-LL-L--L-LL-L--
    -R--R-RR-R--R-RR-R--R-RR
    B--B--B--B--B--B--B--B--
    H---H---H---H---H---H---
    (the triplet paradiddle with high-hat)