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.)
foreach (@ARGV) {
usagedie() if (!-f || !-d);
}
I'd prefer to write this as
usagedie() if grep not( -f or -d ), @ARGV;
The following is needlessly repetitive:
find(\&fileop, $opts{'directory'}) unless @ARGV;
find(\&fileop, @ARGV) if @ARGV;
It could be written simpler like so:
find( \&fileop, @ARGV ? @ARGV : $opt_directory );
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.
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);
}
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 {
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;
}
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 like
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.