On Fri, Dec 30, 2011 at 7:11 PM, Brandon McCaig <bamcc...@gmail.com> wrote:
> On Thu, Dec 29, 2011 at 03:43:19PM +0000, Jonathan Harris wrote:
> > Hi All
>
> Hello Jonathan:
>
> (Disclaimer: I stayed up all night playing Skyrim and am running
> on about 4.5 hours of sleep.. ^_^)
>
> I think most things have already been addressed, but I think Igor
> might have had a bit of trouble making it clear.
>
> > opendir (my $in, $path) or die "Cannot open $dir: $!\n";
> > find (\&wanted, $path);
> > close $in;
> >
> > opendir (my $totalin, $path) or die "Cannot open $dir: $!\n";
> > find (\&cleanup, $path);
> > close $totalin;
>
> AFAICT, it's completely nonsensical to open a directory file
> handle surrounding File::Find::find. I tried to /search the
> perldoc (just in case there's some kind of magical optimization
> or something) and saw no mention of 'opendir' or 'handle' (except
> for the special _ file handle created for stat, lstat, etc..). So
> it seems $in and $totalin are completely unnecessary here:
> File::Find will worry about opening and processing the
> directories for you.
>
> > sub wanted {
> > while ($dircontents = readdir($in)) {
>
> I guess this is why you are opening directory handles above, but
> it doesn't really make sense. You're basically only using
> File::Find to loop at this point, and very obscurely. :)
> File::Find's role in life is precisely to find you all the files
> within a directory tree. You're reinventing the square wheel with
> your use of opendir and readdir. :)
>
> The wanted subroutine is typically used to either process the
> file system tree outright, or store applicable files in data
> structures for later processing. E.g.,
>
> use strict;
> use warnings;
> use File::Find;
>
> my @files;
>
> sub wanted
> {
> # Skip dot files and directories.
> return if substr($_, 0, 1) eq '.' || -d $_;
>
> # If current file is a normal file, push into array for
> # later.
> push @files, $File::Find::name if -f $_;
> }
>
> my $path = '.';
>
> find \&wanted, $path;
>
> # Now @files should be filled with a recursive list of files to
> # process. E.g.,
>
> for my $file (@files)
> {
> my $md5name = $file.'md5';
> # Etc...
> }
>
> > my $hex = Digest::MD5->new->addfile($fh)->hex digest;
>
> I assume you meant `hexdigest' here, not 'hex digest'.
>
> > $newname =~ s/\ //;
>
> Ideally if you're going to do something as obscure as this, you
> should comment it in both places so future readers and
> maintainers understand why it's done, even if they only read one
> half of the program.
>
> I think Igor else has already explained how to eliminate this
> obscurity though. :)
>
> Regards,
>
>
> --
> Brandon McCaig <bamcc...@gmail.com> <bamcc...@castopulence.org>
> Castopulence Software <https://www.castopulence.org/>
> Blog <http://www.bamccaig.com/>
> perl -E '$_=q{V zrna gur orfg jvgu jung V fnl. }.
> q{Vg qbrfa'\''g nyjnlf fbhaq gung jnl.};
> tr/A-Ma-mN-Zn-z/N-Zn-zA-Ma-m/;say'
>
>
> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG v1.4.10 (GNU/Linux)
>
> iQIcBAEBAgAGBQJO/gzlAAoJEN2n1gIi5ZPyMJoP/15LI4aelHowMiNQYFzLB2E1
> nEAjPHvvHX7uTxLV8BOht9HtoQPpOwyQ/fJm2EIe59NGwjjvlKCdpCrfkfxz3cvV
> td0wDuUKrjzQPaACl4cNrGITZLVXe6KtZUKG2o3TjA5dlyqbes5d5F40Mh3j/fB5
> L0VZpWNvp0cTrJ6x5QfTkvyQPdxKx0ARaFDQYpvR3uKfgeD28ZNatNCQQuuymLkj
> ABVqLzQgVVMinaj/4xXii5vedgYFI58DPWF7r0nmhUaiVvDAEFzfd1MlSvkuI8Jr
> EKTQdz3EjMZufRaGxX96rdZvVMEiSTcA/IXNkhis48dOwa4ebfSYm8QpQhp1E6UF
> QuJBRXzF9cHvD095Aw+MSqoSR+2LZS3SFBfdB9I5rdxJEoS7LMSJdJLY95dXftqR
> KdlcU7ds4kdqaJrnxrxB05SIhgZNq1JcCk9xZyuk7NxsPwl/6ZStr/E/V2mA+bdD
> bGWCuP7IKRfZ8cNd01tEYnUppYkfxRaNtYhlNmPVh6TX7rd+2Z4aZLn0anR3Q/J4
> 2OsPmtIOakk1jW6f41vTOqZ5cpxSv/R1H6fjZAOVACf7UlrtCgVWVLKYofcO/ryS
> HMsX1T3m3h9H3heSd76FiXMsgDBaMBkti31FzwnS3pOaZEpHk6eHF/dbR2p0sAbA
> gTvG4xDp2MLEtY03Ofhw
> =RFiR
> -----END PGP SIGNATURE-----
>
>
HI Brandon
Thanks for your response
I totally agree with your first point
Having now used File::Find a little more, I have seen that using opendir
was totally unneccessary and have removed them from the script
And guess what.....it works fine without them!
I think that my initial confusion arose from fundamentally misunderstanding
File::Find - thinking that it required a handle, not just a path.
I have also now exchanged the while loop with a foreach loop - much better!
>> I assume you meant `hexdigest' here, not 'hex digest'.
You assume correctly! Gmail has started to do annoying auto text complete -
I must turn it off!!
>> push @files, $File::Find::name if -f $_;
This is nice and clean
Your approach is different to what we have been discussing
You seem to gather the files with File::Find and then leave that sub alone
asap
The processing is then done in the results of that gathering
My script left the processing within sub wanted
This could possible be a reason that complications arose so quickly
To get file names and sizes at the same time, I am also considering
my %files;
sub wanted {
my $filesize = (stat($_))[7];
push @{$files{$filesize}}, $File::Find::name;
}
find(\&wanted, $path);
to hash files and file size results together - then process after
And yep, Igor has been thorough and very helpful
Thanks again for your input on this - hope you manage to get some sleep!
All the best
Jonathan
