First, there's a lot of repetition there that can be removed by setting your @list variable to the "concatenation" of the lists returned by the glob. Second, you grab $result but you never check whether the move was successful! I'd think about rewriting the chunk above as:my @list = glob("$home/*.$type"); foreach my $file (@list) { my $result = `mv $file $home/$basedir/$key`; } $type = uc($type); @list = glob("$home/*.$type"); foreach $file (@list) { $result = `mv $file $home/$basedir/$key`; }
(the \U modifier does the uc translation within double quoted strings).foreach my $file (glob("$home/*.$type"), glob("$home/*.\U$type")) { system('mv', $file, "$home/$basedir/$key") == 0 or warn "Could not move $file: $!"; }
Also, you could do to a test for existence of directories (-d $dirname, I believe) and create those on the fly rather than forcing their existence beforehand, something like
Here I presume a *nix environment, which seems likely given your earlier use of 'mv'. Natch, you should test the return value of that system call, too....system('mkdir', '-p', "$home/$basedir/$key") unless -d "$home/$basedir/$key";
In reply to Re: tidyhome
by snax
in thread tidyhome
by billysara
| For: | Use: | ||
| & | & | ||
| < | < | ||
| > | > | ||
| [ | [ | ||
| ] | ] |