in reply to How would you go about it?
Check out The Dynamic Duo --or-- Holy Getopt::Long, Pod::UsageMan! and GetOpt::Long usage style. Put together they're how I do all my script writing.
Here are some minor points I'd do differently. (I'll be using my variables instead of your $opts hash, because that affords me the safety of strict. See aforementioned link.)
I'd prefer to write this asforeach (@ARGV) { usagedie() if (!-f || !-d); }
The following is needlessly repetitive:usagedie() if grep not( -f or -d ), @ARGV;
It could be written simpler like so:find(\&fileop, $opts{'directory'}) unless @ARGV; find(\&fileop, @ARGV) if @ARGV;
In this part you're checking -f and -d three or four times per entry, and you're repeating yourself in the conditionals. You also print the directory or file names, even if the corresponding mode option was not set, and they were therefor not touched — probably a minor bug.find( \&fileop, @ARGV ? @ARGV : $opt_directory );
How about the following? It's longer, but less redundant — there is only one location at which to update or add any single condition.sub fileop { #set the right permissions based on if a file or a directory #only set permissions of the mode is set chmod oct($opts{'filemode'}), $_ if -f && $opts{'filemode'}; chmod oct($opts{'dirmode'}), $_ if -d && $opts{'dirmode'}; print $File::Find::name . "\n" if $opts{'verbose'} && (-f || -d); }
Finally, instead of pulling the values through oct every time through the function, I'd do that once at the beginning of the script, as part of input validation that should check whether they're valid modes. Something likesub fileop { if( -f and $opt_filemode ) { chmod oct( $opt_filemode ), $_; } elsif( -d and $opt_dirmode ) { chmod oct( $opt_dirmode ), $_; } else { return; # avoid falling thru to print() } print $File::Find::name, "\n" if $opt_verbose; }
sub check_perm { my ( $mode, $perm ) = @_; if( defined $perm ) { return ( $perm =~ /^[0-7]{3}$/ ) ? oct( $perm ) : die "Malformed $mode mode $perm\n"; } return; } $opt_filemode = check_perm file => $opt_filemode; $opt_dirmode = check_perm directory => $opt_dirmode;
Makeshifts last the longest.
|
|---|
| Replies are listed 'Best First'. | |
|---|---|
|
Re^2: How would you go about it?
by pcassell (Sexton) on Jul 26, 2004 at 18:41 UTC | |
by Aristotle (Chancellor) on Jul 26, 2004 at 18:52 UTC |