Hi Jonathan,
Let's review your script a bit, shall we? )
It's definitely good for a starter, but still has some rough places.
#!/usr/bin/perl
> # md5-test.plx
> use warnings;
> use strict;
>
use File::Find;
>
use Digest::MD5;
> use File::Spec;
>
So far, so good. )
> my $dir = shift || '/Users/jonharris/Documents/begperl';
>
Nice touch, setting up a default param. ) The name of variable might seem
too generic to some, but then again, it's the only directory we deal with,
so...
> my ($dircontents, $path, @directory, $fh, $wr_fh);
>
Incoming!
Well, it's usually better to declare your variables right before you'll
really need them...
Your script is short, so you'll hardly have a chance to forget what $fh and
$wr_fh mean, though. )
> @directory = $dir;
> $path = File::Spec->catfile( @directory, $dircontents );
>
Ahem. At least three 'wtf' moments for me. )
First of all, File::Spec->catfile is really just a glorified join operator
with some additional operations depending on which system you're using.
So, second, it makes a little sense to convert $dir into @directory
(documentation example is just that, an example) and to pass there
undefined $dircontents as well.
But the major one is why you'd ever have to pass your $dir through
File::Spec? It's, you know, user input... )
opendir (my $in, $path) or die "Cannot open $dir: $!\n";
>
So you're trying to open $path, but warn about failure to open $dir? )
But then again, that's a minor quarrel, considering this:
> find (\&wanted, $path);
>
See, File::Find is convenient method which _emulates_ the whole
opendir-readdir-closedir pattern for a given directory.
The 'wanted' subroutine (passed by ref) will be called for each file found
in $path.
It's described really well in perldoc (perldoc File::Find).
close $in;
>
Opendir, but close - and not closedir? Now I'm confused. )
opendir (my $totalin, $path) or die "Cannot open $dir: $!\n";
> find (\&cleanup, $path);
> close $totalin;
>
You don't have to use different variable to store temporary file handle
(which will be closed in three lines) - and that will save you a few
moments spent working out a new (but rational) name for it. :)
But then again, you don't need to open the same dir twice: you can call
cleanup (with the same 'find (\&cleanup)... ' syntax) whenever you want.
And you don't really need cleanup... whoops, going too far too soon. :)
print "\n\nAll Done!\n\n";
>
> sub wanted {
> while ($dircontents = readdir($in)) {
>
Did I say that you're using two alternative methods of doing the same
thing? )
But there's another big 'no-no' here: you're using external variable
($dircontents) when you really have perfectly zero reasons to do so.
Of course, you don't need to push dirhandle ($in) from outer scape into
this sub, when using find... ($File::Find::dir will do), but that's
explainable at least. )
if ($dircontents=~/^\./ || -d $dircontents) {
> next;
> }
>
So now the script ignores all the files which names begin with '.', and you
really wanted just to ignore '.' and '..' ... )
my $bytes = -s $dircontents;
>
print $dircontents, "\n";
> print $bytes, " bytes", "\tSo far so good!\n";
>
Yes. )
> open $fh, "<", "$dircontents" or die $!;
> open $wr_fh, "+>", "$path $dircontents.md5" or die $!; ## was unable to
> concatenate here, hence sub cleanup to remove the ' '
>
What was wrong with ...
open my $wr_fh, '>', File::Spec->catfile($path, $dircontents, '.md5') or
die $!, $/
?
>
> my $hex = Digest::MD5->new->addfile($fh)->hex digest;
> print "Hex Digest: ", $hex, "\n\n";
> print $wr_fh $hex, "\n", $bytes, "\n\n";
>
All looks great for now: you're calculating md5 and size, and writing it
into file with md5 extension...
> return($hex);
>
... but now you're abruptly jumping out of the while block, making the
whole point of cycle, well, completely pointless. Not great.
> close $wr_fh;
> close $fh;
>
}
> }
>
>
> # The following is mostly not original code - thanks to the author!
>
sub cleanup {
> my @filelist = readdir($totalin);
> foreach my $oldname (@filelist) {
> next if -d $oldname;
> my $newname = $oldname;
> $newname =~ s/\ //;
>
So you don't have spaces in your filenames. Great for you. )
> rename $oldname, $newname;
> }
> }
>
>
##### End #####
>
>
And here we finish.
Computers are not smart. They're dumb. But they're fast. And obedient. )
That's why they're really helpful in letting you do what you're trying to
do... but only if you KNOW what you're trying to do.
Imagine that you're - and not your computer - will be doing this task.
Sit in one place - and write down your program as you (and not your
computer) will be running it. Step by step. Bit by bit.
Then convert your notes into some Perl form - and you'll instantly see the
difference between now and then. )
-- iD
